-
Notifications
You must be signed in to change notification settings - Fork 42
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
Updated CI workflows to avoid race conditions on PRs with concurrency label #778
Conversation
Hello. You may have forgotten to update the changelog!
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #778 +/- ##
=======================================
Coverage 98.48% 98.48%
=======================================
Files 109 109
Lines 15801 15801
=======================================
Hits 15562 15562
Misses 239 239 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but do we want to target the RC branch?
Probably best to have it in the default branch first. I'll open a subsequent PR to the rc branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Don't forget to update the changelog.
…AI/pennylane-lightning into sc-66955-fix-ci-race-condition
…AI/pennylane-lightning into sc-66955-fix-ci-race-condition
Re-requesting a review as I've had to address an additional issue within the PR. Scipy was being installed to the python venv on the self-hosted runner, but the build was using the system Python (and thus could not find the installed scipy). This was causing sporradic failures and bugs. The solution was to export |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks again @rashidnhm .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @rashidnhm! Looks good to merge now :)
Context:
Currently there exists a race condition within the CI due to misconfiguration of the concurrency label.
This PR updates the affected workflows to fix the concurrency labels.
Description of the Change:
${{ github.event }}
is not a string, but rather an object, when you have that in the concurrency label you get:This was causing workflows that had
pull_request
andworkflow_call
triggers to overstep on each other's runtime as they both rendered with the same concurrency label.This PR changes the attribute to use the correct string name
event_name
.Benefits:
Race conditions averted. No random job cancellations especially when PR needs wheels built.
Possible Drawbacks:
None.
Related GitHub Issues:
None. sc-66955