Skip to content

Attempt to change Push CI to workflow_run#17753

Merged
ydshieh merged 8 commits intomainfrom
push_ci_on_workflow_run_part_2
Jun 18, 2022
Merged

Attempt to change Push CI to workflow_run#17753
ydshieh merged 8 commits intomainfrom
push_ci_on_workflow_run_part_2

Conversation

@ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Jun 17, 2022

What does this PR do?

This is a fix for #17692 :

  • Use the correct properties to get the information (branch/commit-SHA, etc.) for both push and workflow_run event type
    • Even if we consider only the workflow_run event triggered by a push to main branch, we still need to use github.event.workflow_run.head_sha to get the correct SHA (otherwise, in the case where 2 PRs merged into main in a very short time period, the first one will get the latest commit SHA)
    • Currently, the push CI could still be triggered by push event if the branch is NOT main. The main purpose is for testing a particular branch. This is why I need to consider both event type.

NOTE: I have verified the change extensively in my own (dummy) repo. However, the part regarding the actual CI tests + the part of slack report are not verified. The part regarding preparing the necessary information for slack report is verified. Since the workflow_run could be launched only when the PR is merged into main, I hope there is no other unexpected issue in this PR 🙏.

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Jun 17, 2022

The documentation is not available anymore as the PR was closed or merged.

@ydshieh ydshieh requested a review from sgugger June 17, 2022 12:56
Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@ydshieh
Copy link
Collaborator Author

ydshieh commented Jun 17, 2022

I will merge this tomorrow (so unlikely to have other PRs merged), and revert it if anything breaks (hopefully not, I am running out of ideas 😄 )

@ydshieh ydshieh merged commit 6589e51 into main Jun 18, 2022
@ydshieh ydshieh deleted the push_ci_on_workflow_run_part_2 branch June 18, 2022 06:35
@ydshieh
Copy link
Collaborator Author

ydshieh commented Jun 20, 2022

This works well, one good example to check is

TF: BART compatible with XLA generation

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