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

Add paths filter to avoid the chance of being triggered #30453

Merged
merged 2 commits into from
Apr 24, 2024

Conversation

ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Apr 24, 2024

What does this PR do?

We decide to remove the status check from the repository setting, i.e. no more Check slow test status as in the following screenshot.

Therefore, we don't need to trigger the workflow file all the time. This PR adds a paths filter. The workflow will be triggered only if there is a change matching src/transformers/models/*/modeling_*.py.

  • For external contributors' PRs, this avoids to click unnecessary Approve and run most of the time.
  • If a change to modeling files, the workflow will still be trigger even if it is not a new model PR.
    • If it is from external contributor, Approve and run would be shown and waits for click.
    • If it is NOT new model PR, reviewers could ignore it however. But if it is clicked it (after review), it will be ✅ anyway
  • If it is a new model PR, the slow tests will be triggered (external PRs need the button to be clicked + assume the required label single-model-run-slow is added), and the reviewers can check it is ✅ or ❌ (which is easy to identify).
Screenshot 2024-04-24 100723

@ydshieh ydshieh force-pushed the improve_new_model_pr_ci branch from d8bdddd to a3e01fc Compare April 24, 2024 12:26
@ydshieh ydshieh requested a review from amyeroberts April 24, 2024 12:30
@HuggingFaceDocBuilderDev

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Copy link
Collaborator

@amyeroberts amyeroberts left a comment

Choose a reason for hiding this comment

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

Thanks!

Comment on lines -111 to -125

slow_test_result:
runs-on: ubuntu-22.04
name: Check slow test status
needs: [check_for_new_model, run_new_model_tests]
if: always()
steps:
- name: Check test status
shell: bash
# NOT a new model PR --> pass
# new model PR --> pass only if `run_new_model_tests` gives `success` (so if the label is not added, we fail
# this job even if `run_new_model_tests` has `skipped` status).
run: |
echo "${{ needs.run_new_model_tests.result }}"
if [ "${{ needs.check_for_new_model.outputs.new_model }}" = "" ]; then echo "not new model"; elif [ "${{ needs.run_new_model_tests.result }}" != "success" ]; then echo "failure"; exit -1; else echo "pass"; fi;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed this one (forgot to do it in the first commit).

This job was here only to make our status check on the repository setting work - but we no longer use that tool, so this job is no longer useful.

@ydshieh ydshieh merged commit 42fed15 into main Apr 24, 2024
8 checks passed
@ydshieh ydshieh deleted the improve_new_model_pr_ci branch April 24, 2024 14:58
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.

None yet

3 participants