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 support for the pull_request_target event in the upload_pr_documentation workflow #392

Merged
merged 1 commit into from
Aug 24, 2023

Conversation

regisss
Copy link
Contributor

@regisss regisss commented Aug 12, 2023

In Optimum we need to use the pull_request_target event to be able to access repo secrets to build PR docs. However, PR docs have not been uploaded since this change happened.
This PR adds support for this event type in the upload_pr_documentation workflow.

@sgugger
Copy link
Contributor

sgugger commented Aug 13, 2023

cc-ing @coyotte508 as well as he designed the current system with @mishig25

Just adding the workflow won't necessarily give you access to secrets, it's a tiny bit tricky and they should be able to explain it better than me.

@regisss
Copy link
Contributor Author

regisss commented Aug 14, 2023

cc-ing @coyotte508 as well as he designed the current system with @mishig25

Just adding the workflow won't necessarily give you access to secrets, it's a tiny bit tricky and they should be able to explain it better than me.

Actually accessing secrets to build the documentation is not the issue here, sorry I should have been more accurate. PR docs are built properly, the problem is that they are not uploaded because the "Upload PR documentation" workflow is not triggered for pull_request_target events.

For instance, here the PR doc was built properly: https://github.com/huggingface/optimum/actions/runs/5843309088/job/15845336669?pr=1280
But docs are never uploaded: https://github.com/huggingface/optimum/actions/workflows/upload_pr_documentation.yml

@fxmarty
Copy link
Contributor

fxmarty commented Aug 23, 2023

@mishig25 @regisss How does it sound?

@regisss
Copy link
Contributor Author

regisss commented Aug 23, 2023

@mishig25 @regisss How does it sound?

@sgugger @mishig25 I think @coyotte508 is out on paternity leave till October 1st and we need this in Optimum to get the PR doc working, is there any solution before he comes back?

@sgugger
Copy link
Contributor

sgugger commented Aug 23, 2023

You should ping Mishig offline to check out this PR, he doesn't seem to read his GitHub notifications.

Copy link
Contributor

@mishig25 mishig25 left a comment

Choose a reason for hiding this comment

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

lgtm !

@mishig25 mishig25 merged commit efc982f into huggingface:main Aug 24, 2023
4 checks passed
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.

4 participants