Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refine the trigger of net-core-ci #45804

Open
live1206 opened this issue Sep 5, 2024 · 7 comments
Open

Refine the trigger of net-core-ci #45804

live1206 opened this issue Sep 5, 2024 · 7 comments
Labels
Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system.

Comments

@live1206
Copy link
Member

live1206 commented Sep 5, 2024

Library name

N/A

Please describe the feature.

Right now, any change in ./eng apart from the excludes will trigger net-core-ci, which takes quite a while to finish.



exclude:
- eng/mgmt/
- eng/common/

Is it possible to narrow down the trigger further to avoid unnecessary ci runs?

During #45617, I exclude the newly added ./eng/pacakges to avoid later unnecessary net-core-ci trigger.

@github-actions github-actions bot added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Sep 5, 2024
@live1206 live1206 changed the title Narrow down the trigger of net-core-ci Refine the trigger of net-core-ci Sep 5, 2024
@live1206
Copy link
Member Author

live1206 commented Sep 5, 2024

And also, net-core-ci was not triggered during #45605, which did break net-core-ci, filed #45806 for it.
We should add PR trigger of sdk/identity for net-core-ci.

@jsquire jsquire added Client This issue points to a problem in the data-plane of the library. and removed needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. labels Sep 5, 2024
@jsquire
Copy link
Member

jsquire commented Sep 5, 2024

@live1206: The intent for this is to ensure that no change to the engineering system causes a regression that breaks SDK builds. The Core pipeline runs Azure.Core as well as a representative sample of other SDKs. We do not want to lose this validation, as it is an essential one to avoid repository-wide breaks.

Since the pipelines do run code generation in order to do API differences, I'm not sure that I agree we can safely make Azure plugin changes and not potentially break the entire repository. Unless I'm missing something, I don't think we'll want to exclude that.

//cc: @m-nash, @annelo-msft

@jsquire jsquire added EngSys This issue is impacting the engineering system. and removed Azure.Core labels Sep 5, 2024
@annelo-msft
Copy link
Member

I don't have a good sense of the intended process around updates to the Azure plugin yet -- I feel like it would be good to evaluate that request in light of that. For example, when changes to the Azure plugin go in, will they regenerate all relevant Azure clients in the repo as part of the same change? Do we have the intended steps in that process written down somewhere that we could reference?

@live1206
Copy link
Member Author

live1206 commented Sep 5, 2024

And also, net-core-ci was not triggered during #45605, which did break net-core-ci, filed #45806 for it. We should add PR trigger of sdk/identity for net-core-ci.

@jsquire @annelo-msft Shall we at least fix this firstly?

@live1206
Copy link
Member Author

live1206 commented Sep 5, 2024

@live1206: The intent for this is to ensure that no change to the engineering system causes a regression that breaks SDK builds. The Core pipeline runs Azure.Core as well as a representative sample of other SDKs. We do not want to lose this validation, as it is an essential one to avoid repository-wide breaks.

Since the pipelines do run code generation in order to do API differences, I'm not sure that I agree we can safely make Azure plugin changes and not potentially break the entire repository. Unless I'm missing something, I don't think we'll want to exclude that.

Given Azure plugin isn't generating any SDK for now, can we skip Azure plugin during the implementation progress for now? It will save us quite some time to skip net-core-ci check during the PRs, which will be many in the coming months. We will reconsider adding it back or not when we enable SDK generation for it.

I don't have a good sense of the intended process around updates to the Azure plugin yet -- I feel like it would be good to evaluate that request in light of that. For example, when changes to the Azure plugin go in, will they regenerate all relevant Azure clients in the repo as part of the same change? Do we have the intended steps in that process written down somewhere that we could reference?

I agree we should think about the process thoroughly, in my mind it still follows the process of autorest.chsarp, that we need to re-generate all SDKs while we update the generator. I don't have a good answer for it at the current stage, might have better understanding during the implementation. And for sure, we should get this settled when Azure plugin is set to generate SDKs.

@jsquire
Copy link
Member

jsquire commented Sep 5, 2024

Given Azure plugin isn't generating any SDK for now, can we skip Azure plugin during the implementation progress for now? It will save us quite some time to skip net-core-ci check during the PRs, which will be many in the coming months. We will reconsider adding it back or not when we enable SDK generation for it.

No objection to a short-term work-around for implementation - so long as this doesn't bypass validation when we start using the plugin for real-world generation in the repo. Let's please have an issue tracking reenabling it and figure out how to keep visibility there so that we don't overlook it.

@live1206
Copy link
Member Author

live1206 commented Sep 6, 2024

Given Azure plugin isn't generating any SDK for now, can we skip Azure plugin during the implementation progress for now? It will save us quite some time to skip net-core-ci check during the PRs, which will be many in the coming months. We will reconsider adding it back or not when we enable SDK generation for it.

No objection to a short-term work-around for implementation - so long as this doesn't bypass validation when we start using the plugin for real-world generation in the repo. Let's please have an issue tracking reenabling it and figure out how to keep visibility there so that we don't overlook it.

Created #45825 for tracking to re-enable it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. EngSys This issue is impacting the engineering system.
Projects
None yet
Development

No branches or pull requests

3 participants