Skip to content

Avoid using context that is not accessable from external contributors#33866

Merged
ydshieh merged 4 commits intomainfrom
fix_circleci_issue
Oct 1, 2024
Merged

Avoid using context that is not accessable from external contributors#33866
ydshieh merged 4 commits intomainfrom
fix_circleci_issue

Conversation

@ydshieh
Copy link
Collaborator

@ydshieh ydshieh commented Oct 1, 2024

What does this PR do?

@HuggingFaceDocBuilderDev

Hey! 🤗 Thanks for your contribution to the transformers library!

Before merging this pull request, slow tests CI should be triggered. To enable this:

  • Add the run-slow label to the PR
  • When your PR is ready for merge and all reviewers' comments have been addressed, push an empty commit with the command [run-slow] followed by a comma separated list of all the models to be tested, i.e. [run_slow] model_to_test_1, model_to_test_2
    • If the pull request affects a lot of models, put at most 10 models in the commit message
  • A transformers maintainer will then approve the workflow to start the tests

(For maintainers) The documentation for slow tests CI on PRs is here.

Comment on lines +195 to 202
setup_and_quality_2:
when:
not:
equal: [<<pipeline.project.git_url>>, https://github.com/huggingface/transformers]
jobs:
- check_circleci_user
- check_code_quality
- check_repository_consistency
- fetch_tests:
# [reference] https://circleci.com/docs/contexts/
context:
Copy link
Collaborator Author

@ydshieh ydshieh Oct 1, 2024

Choose a reason for hiding this comment

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

This is for private repositories.

(i.e. for some model additions)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think this is kind of what Henna suggested in her message. As external contributors won't have permission to access TRANSFORMERS_CONTEXT, we should either remove the context: or remove the private contents that users can't access.

when:
not: <<pipeline.parameters.nightly>>
and:
- equal: [<<pipeline.project.git_url>>, https://github.com/huggingface/transformers]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if it is huggingface/transformers : don't use context!

Comment on lines +198 to +201
setup_and_quality_2:
when:
not:
equal: [<<pipeline.project.git_url>>, https://github.com/huggingface/transformers]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

for private forked repositories (e.g. some model additions): use context

Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

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

Cool, so for private fork, we go though the non-context, will it still work? 🤗

@ydshieh
Copy link
Collaborator Author

ydshieh commented Oct 1, 2024

Cool, so for private fork, we go though the non-context, will it still work? 🤗

public (i.e. hf/transformers): go without context --> works as it doesn't require the token to get the artifacts
private: go with context --> works because it has the required token and it's definitely our team members on it

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.

Thanks a lot Yih-Dar!

@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.

@ydshieh ydshieh merged commit f205da9 into main Oct 1, 2024
@ydshieh ydshieh deleted the fix_circleci_issue branch October 1, 2024 15:42
@guangy10
Copy link
Contributor

guangy10 commented Oct 1, 2024

@ydshieh Thanks for the fix!

BernardZach pushed a commit to BernardZach/transformers that referenced this pull request Dec 5, 2024
…huggingface#33866)

* fix

* fix

* fix

* fix

---------

Co-authored-by: ydshieh <ydshieh@users.noreply.github.com>
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