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

Restricted AArch64 CI to relevant changes #2259

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vpirogov
Copy link
Member

There's no need to run AArch64 CI on changes that are unlikely to affect this platform.

@vpirogov vpirogov requested a review from a team as a code owner December 12, 2024 23:22
@github-actions github-actions bot added the devops Github automation label Dec 12, 2024
@vpirogov vpirogov marked this pull request as draft December 12, 2024 23:23
@vpirogov
Copy link
Member Author

The issue with approach taken in this PR is that Github still considers AArch64 CI checks required and prevents PR from being promoted.

@vpirogov
Copy link
Member Author

@theComputeKid, any ideas here?

@theComputeKid
Copy link
Contributor

One solution (but not ideal) is that we add a step in the AArch64 pipeline to read the label and then make a decision based on that to quit successfully early.

Drawbacks:

  1. We will still spin up an instance for a few seconds
  2. It will still book a slot in the Q
  3. It will label the AArch64 as passed even though it was skipped

Advantages:

  1. We will only spin up an instance for a few seconds instead of the whole run.

I'm working on a PR to reduce the configs AArch64, should have that out shortly.

@mgouicem
Copy link
Contributor

Stackoverflow suggests to create an action with same name but with opposite condition that would always run successfully (see here)

@vpirogov
Copy link
Member Author

vpirogov commented Dec 13, 2024

Stackoverflow suggests to create an action with same name but with opposite condition that would always run successfully (see here)

This approach is very limited as 'opposite condition' cannot always be described using trigger path syntax. Conditions suggested in the answer with path: xxx and path-ignore: xxx are not mutually exclusive. A PR touching files matching xxx and not matching xxx will trigger both workflows. This is discussed further down the thread.

@vpirogov vpirogov force-pushed the vpirogov/ci-restrict branch from 74a4569 to 571c69f Compare December 18, 2024 01:49
@vpirogov vpirogov changed the title github: workflows: excluded cpu/x64 and gpu from AArch64 CI trigger Restricted AArch64 CI to relevant changes Dec 18, 2024
@vpirogov
Copy link
Member Author

I would suggest that at this point we make sure that AArch64 CI runs only for relevant changes and figure out how to make it required later. Github developers are aware of our pain.

We are taking the same approach for x64 CI: #2281. I expect codeowners (i.e. people allowed to merge PRs) would use CI as a tool to prevent breaking changes.

@vpirogov vpirogov marked this pull request as ready for review December 18, 2024 01:55
@vpirogov vpirogov force-pushed the vpirogov/ci-restrict branch 8 times, most recently from bbec023 to 430aba3 Compare December 18, 2024 05:12
@vpirogov vpirogov force-pushed the vpirogov/ci-restrict branch from 430aba3 to 3026439 Compare December 18, 2024 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devops Github automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants