Conversation
Micky774
requested review from
ipanfilo,
wangye805 and
wenchenvincent
as code owners
ipanfilo
left a comment
ipanfilo
left a comment
There was a problem hiding this comment.
Maybe make approach more generic - ci_level% label specifies CI level to use.
Micky774
changed the title
|
|
||
| // Check if the removed level was higher than the current level | ||
| const currentLevel = level ? Number(level) : 0; | ||
| const removedWasHighest = removedLevel > 0 && removedLevel > currentLevel; |
There was a problem hiding this comment.
NIT: removedLevel > 0 is redundant
| pull_request: | ||
| branches: | ||
| - 'dev' | ||
| - 'release_v1.**_rocm' |
There was a problem hiding this comment.
NIT: release_v1 can be removed. We hardly support GHA for them now
| @@ -0,0 +1,135 @@ | |||
| name: PR Automatic CI | |||
| const toCancel = runs.data.workflow_runs.filter(run => { | ||
| const prMatch = (run.pull_requests || []).some(pr => pr.number === prNumber); | ||
| const active = run.status === 'queued' || run.status === 'in_progress'; | ||
| if (action === 'synchronize') { |
There was a problem hiding this comment.
reopened action does not seem handled in this code
| // are queued or in progress, and have a lower CI level | ||
| const toCancel = runs.data.workflow_runs.filter(run => { | ||
| const prMatch = (run.pull_requests || []).some(pr => pr.number === prNumber); | ||
| const active = run.status === 'queued' || run.status === 'in_progress'; |
There was a problem hiding this comment.
You can return false here if not prMatch or not active so do not need recheck it later. And, probably if run.id === currentRunId
| // If a label was added, cancel runs with a lower level than the added label | ||
| if (action === 'labeled') { | ||
| const isLowerThanAdded = runLevel > 0 && runLevel < addedLevel; | ||
| return prMatch && active && isLowerThanAdded && run.id !== currentRunId; |
There was a problem hiding this comment.
can is LowerThanAdded be if run.id === currentRunId?
| .map(label => label.name.toLowerCase()); | ||
|
|
||
| // Determine if a CI level label was removed and what level it was | ||
| const removedLabel = (context.payload.label?.name || '').toLowerCase(); |
There was a problem hiding this comment.
Why is it removedLabel? For labeled event it is addedLabel.
Maybe unlabeled events are not needed at all? If higher lablel is added - CI with new level is triggered. Otherwise the last CI run should still be relevant.
There was a problem hiding this comment.
If a PR has labels ci-level 1 and ci-level 3 but then ci-level 3 is removed, then we still need to run ci-level 1 tests.
| # See LICENSE for license information. | ||
|
|
||
| name: TransformerEngine CI | ||
| name: Test and Build Branch |
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.ref }} | ||
| cancel-in-progress: true | ||
| group: ${{ github.event_name == 'workflow_call' && format('{0}-{1}-{2}', github.workflow, github.ref, github.run_id) || format('{0}-{1}', github.workflow, github.ref) }} |
There was a problem hiding this comment.
Why modified group is needed for workflow_call ?
2 participants
Description
With this PR, testing is mediated by labels such as:
ci-level {1, 2, 3}. No tests will be run without at least one label present. At any given time, the highest-level label will take precedence, and only its corresponding level of tests will be run. The workflow triggers on labeling, unlabeling, and pushing commits. When removing a label, the workflow will check to see if there are any labels remaining and if there are, it will dispatch to the highest of them.Fixes # (issue)
Type of change
Changes
Please list the changes introduced in this PR:
Checklist: