Skip to content

[CI] Switch pre-commit to a new scheme #10720

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

Merged
merged 3 commits into from
Aug 8, 2023

Conversation

aelovikov-intel
Copy link
Contributor

Use pull_request trigger (instead of pull_request_target) for
everything except AWS CUDA E2E testing. The latter has to go to a
separate workflow (workflow_run) in order to have access to the AWS EC
key kept as a github secret.

As part of the changes, I also stopped using matrix generator for the
pre-commit task. Instead, the matrix is written directly inside the
task's .yml file. The only minor difference in the behavior is that
driver installation happens on an image with previous driver installed,
not on a system without any driver.

Use pull_request trigger (instead of pull_request_trigger) for
everything except AWS CUDA E2E testing. The latter has to go to a
separate workflow (workflow_run) in order to have access to the AWS EC
key kept as a github secret.

As part of the changes I also stopped using matrix generator for the
pre-commit task. Instead, the matrix is written directly inside the
task's .yml file. The only minor difference in the behavior is that
driver installation happens on an image with previous driver installed,
not on a system without any driver.
@aelovikov-intel aelovikov-intel requested a review from a team as a code owner August 7, 2023 19:32
@aelovikov-intel
Copy link
Contributor Author

Tested in #10715.

Copy link
Contributor

@stdale-intel stdale-intel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM and looking forward to the changes this enables

@aelovikov-intel aelovikov-intel temporarily deployed to aws August 7, 2023 19:44 — with GitHub Actions Inactive
@aelovikov-intel aelovikov-intel temporarily deployed to aws August 7, 2023 20:44 — with GitHub Actions Inactive
@aelovikov-intel aelovikov-intel merged commit 319f067 into sycl Aug 8, 2023
@aelovikov-intel aelovikov-intel deleted the sycl-devops-pr/new-pre-commit branch August 8, 2023 14:51
aelovikov-intel added a commit that referenced this pull request Aug 9, 2023
aelovikov-intel added a commit that referenced this pull request Aug 10, 2023
mdtoguchi pushed a commit to mdtoguchi/llvm that referenced this pull request Oct 18, 2023
Use pull_request trigger (instead of pull_request_target) for
everything except AWS CUDA E2E testing. The latter has to go to a
separate workflow (workflow_run) in order to have access to the AWS EC
key kept as a github secret.

As part of the changes, I also stopped using matrix generator for the
pre-commit task. Instead, the matrix is written directly inside the
task's .yml file. The only minor difference in the behavior is that
driver installation happens on an image with previous driver installed,
not on a system without any driver.
mdtoguchi pushed a commit to mdtoguchi/llvm that referenced this pull request Oct 18, 2023
aelovikov-intel added a commit to aelovikov-intel/llvm that referenced this pull request Jan 23, 2025
That input parameter was introduced to `sycl-windows-build.yml` in
intel#16560 but it's unclear why.

Note that linux version has had it for quite some time, but we started
unsetting it explicitly in intel#10720. The
reason we had it for Linux in the first place is because `GITHUB_SHA`
pointed to the PR's source branch for the `pull_request_target` trigger
that we used at that time. With switching to `pull_request` trigger it
became unnecessary as `GITHUB_SHA` points to the merge commit already.

This change should fix current race condition when a post-commit CI job
can test change landed after the current commit if there is a delay with
runner allocation.
aelovikov-intel added a commit that referenced this pull request Jan 23, 2025
…16757)

That input parameter was introduced to `sycl-windows-build.yml` in
#16560 but it's unclear why.

Note that linux version has had it for quite some time, but we started
unsetting it explicitly in #10720. The
reason we had it for Linux in the first place is because `GITHUB_SHA`
pointed to the PR's source branch for the `pull_request_target` trigger
that we used at that time. With switching to `pull_request` trigger it
became unnecessary as `GITHUB_SHA` points to the merge commit already.

This change should fix current race condition when a post-commit CI job
can test change landed after the current commit if there is a delay with
runner allocation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants