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

Separate Push CI images from Scheduled CI #19170

Merged
merged 2 commits into from
Sep 26, 2022
Merged

Separate Push CI images from Scheduled CI #19170

merged 2 commits into from
Sep 26, 2022

Conversation

ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Sep 23, 2022

What does this PR do?

⚠️ Before merge, I need to build the new push CI images that have the new tags.

Currently, if setup.py is changed, Push CI will re-build the CI images before running tests.

build-docker-containers:
needs: check-for-setup
if: (github.event_name == 'push') && (needs.check-for-setup.outputs.changed == '1')
uses: ./.github/workflows/build-docker-images.yml
secrets: inherit

However, this may cause different jobs in a scheduled CI workflow run to use images with different versions.

Recently, when tokenizers is changed to 0.13, some jobs failed in the scheduled CI due to the new image (with tokenizers 0.13) but the transformers code in those runs still required tokenizers < 0.13.

This PR separates the push CI images from scheduled CI.

@ydshieh ydshieh requested review from LysandreJik, sgugger and muellerzr and removed request for LysandreJik and sgugger September 23, 2022 14:48
Copy link
Contributor

@muellerzr muellerzr left a comment

Choose a reason for hiding this comment

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

Great solution! Makes sense, lg2m

@HuggingFaceDocBuilderDev
Copy link

HuggingFaceDocBuilderDev commented Sep 23, 2022

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

Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

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

Good idea! Let's document that somewhere :)

@ydshieh
Copy link
Collaborator Author

ydshieh commented Sep 23, 2022

Added in Transformers testing design, internal document on Notion.

Screenshot 2022-09-23 191436

Text version

The CI for a push event (to main branch) will check if setup.py is changed. If yes, it will launch the docker image build CI before launching the actual tests. This is to make sure the tests will run against the specified package versions in setup.py. In order to avoid the conflict with the daily schedule CI, which should use the same image version for all jobs during a workflow run, we separate the CI images used for push events and schedule CI. The docker images used for push events start with the tag of the images used in the corresponding jobs in scheduled CI, but with a postfix push-ci. For example, transformers-all-latest-gpu in schedule CI will be transformers-all-latest-gpu-push-ci in push CI.

@ydshieh ydshieh changed the title [Don't merge] Separate Push CI images from Scheduled CI Separate Push CI images from Scheduled CI Sep 26, 2022
@ydshieh ydshieh merged commit 71fc331 into main Sep 26, 2022
@ydshieh ydshieh deleted the separate_ci_images branch September 26, 2022 08:55
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.

5 participants