MNT Fixes doc building on PR #23508
MNT Fixes doc building on PR #23508
Conversation
|
On the CI on main, the circleci upload returns "Project not found". Edit: I fixed it. It turns out, circleci v2 API requires a personal token. |
|
Looking at how ✓ test_circleci_doc_building Documentation builder #23508 · 2418955303
Triggered via pull_request about 1 hour agoIt uses a graphql query based on the Moving forwardWe get this to work with import os
from github import Github
gh = Github(os.environ["GITHUB_TOKEN"])
sk_repo = gh.get_repo("scikit-learn/scikit-learn")
run = sk_repo.get_workflow_run(2418955303)
head = f"{run.head_repository.owner.login}:{run.head_branch}"
prs = list(sk_repo.get_pulls(state='open', sort='created', head=head))
pr = prs[0]
pr.number
# 23508 |
|
Based on https://github.com/scikit-learn/scikit-learn/runs/6683078988?check_suite_focus=true I think this needs to be merged to test that it works. |
One of the main drawbacks of the |
|
Let me try the solution proposed in my |
There was a problem hiding this comment.
Thanks @thomasjpfan for solving this issue. I have tried it in my own repository and is working so, LGTM.
Minor comments.
|
|
||
| gh = Github(args.token) | ||
| sk_repo = gh.get_repo(args.repo) | ||
| run = sk_repo.get_workflow_run(args.run_id) |
There was a problem hiding this comment.
Because they are asserting that id is of integer data type.
| run = sk_repo.get_workflow_run(args.run_id) | |
| run = sk_repo.get_workflow_run(int(args.run_id)) |
There was a problem hiding this comment. 8000
Added the type information in 568b971 (#23508)
| @@ -0,0 +1,17 @@ | |||
| import argparse | |||
There was a problem hiding this comment.
Is this file supposed to be in the maint_tools folder in the build_tools/github one?
There was a problem hiding this comment.
I moved the file to build_tools/github.
|
Just curious, what is the plan for the doc artifact redirector CI status, to be able to check the rendered doc in one click? It seems you need a CircleCI job name for |
@lesteve Looking at the PR in @alfaro96's fork: alfaro96#1 the redirector works without any changes. From the redirector point of view everything is the same.
Looking at the fork, the CircleCI jobs appear after it is triggered by GHA. |
That is. We trigger the same jobs in |
|
BTW, the documentation is correctly pushed to the |
|
OK great! |
|
I updated this PR with a more simple curl command to get the PR number. I tested it out here: https://github.com/thomasjpfan/scikit-learn/runs/6692461745?check_suite_focus=true and it triggers circleci correctly. |
|
You can also see the CircleCI build running on my fork's PR. |
There was a problem hiding this comment.
Thanks again @thomasjpfan; I prefer this more straightforward method. I confirm the URL that is supposed to be used for this pull request contains the necessary information.
LGTM after checking the suggested changes.
| @@ -0,0 +1,22 @@ | |||
| """Gets the Pull Request Number from a workflow's run ID. | |||
There was a problem hiding this comment.
This file is not required anymore, right?
There was a problem hiding this comment.
Yea, this file is not needed anymore. I removed it from the PR.
|
@glemaitre @jeremiedbb @lesteve This PR is required so that circleci in PRs works correctly. I tested it out in thomasjpfan#111 and it should work. |
|
Merging this one, thanks! I don't really see why PULL_REQUEST_NUMBER is not set, as already asked in #23508 (comment) but oh well ... |
This is probably this: https://github.community/t/pull-request-attribute-empty-in-workflow-run-event-object-for-pr-from-forked-repo/154682
This would explain why it was working when @alfaro96 was testing this on its fork. The work-around they found seems a bit more complicated than what is implemented in this PR: |
Fixes #23509
This PR is testing out that doc building works as expected on a PR.
Doc building has been moved to GitHub actions: #21137