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 doc preview CI #380

Merged
merged 4 commits into from
Jan 16, 2025
Merged

Add doc preview CI #380

merged 4 commits into from
Jan 16, 2025

Conversation

leofang
Copy link
Member

@leofang leofang commented Jan 12, 2025

Close #346.

This PR adds a new doc_preview action that deploys docs built in each PR to

https://nvidia.github.io/cuda-python/pr-preview/pr-<id>/

and post a message. Upon PR merge, the action would clean up the pushed files to remove the preview.

The doc_preview largely follows the logic of pr-preview-action, but has to be re-implemented to cope with two limitations:

  1. Our copy-pr-bot can only be triggered by a push event, not by pull_request or pull_request_target events as required by pr-preview-action
  2. The upstream pr-preview-action has no plan to support taking a PR number as an input parameter to bypass the first limitation (see Add doc preview CI #380 (comment))

It is understandable why pull_request* events are preferred, because with push events it is very tricky to get the PR number robustly, both during the PR test stage and during the merge to the main branch, and in the end this is what we need:

@leofang leofang self-assigned this Jan 12, 2025
Copy link
Contributor

copy-pr-bot bot commented Jan 12, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@leofang leofang added documentation Improvements or additions to documentation P1 Medium priority - Should do CI/CD CI/CD infrastructure labels Jan 12, 2025
@leofang
Copy link
Member Author

leofang commented Jan 12, 2025

/ok to test

@leofang leofang marked this pull request as draft January 12, 2025 04:17
@leofang
Copy link
Member Author

leofang commented Jan 12, 2025

Looks like we need the org admin to approve another action. Waiting for response.

@leofang
Copy link
Member Author

leofang commented Jan 13, 2025

Looks like we need the org admin to approve another action. Waiting for response.

Admin has approved. Let's retry...

@leofang
Copy link
Member Author

leofang commented Jan 13, 2025

/ok to test

1 similar comment
@leofang
Copy link
Member Author

leofang commented Jan 13, 2025

/ok to test

@leofang leofang closed this Jan 13, 2025
@leofang leofang reopened this Jan 13, 2025
@leofang
Copy link
Member Author

leofang commented Jan 13, 2025

/ok to test

@leofang
Copy link
Member Author

leofang commented Jan 13, 2025

The action worked, but the way we launched it is not right. It couldn't get the PR number, so

@leofang
Copy link
Member Author

leofang commented Jan 14, 2025

It couldn't get the PR number

Discussed with the upstream maintainer (rossjrw/pr-preview-action#101), and unfortunately we'll have to be creative here. Luckily we have most things set up in the doc-build workflow, so we just need to check in the doc artifacts to the gh-pages branch and push.

@leofang
Copy link
Member Author

leofang commented Jan 16, 2025

/ok to test

4 similar comments
@leofang
Copy link
Member Author

leofang commented Jan 16, 2025

/ok to test

@leofang
Copy link
Member Author

leofang commented Jan 16, 2025

/ok to test

@leofang
Copy link
Member Author

leofang commented Jan 16, 2025

/ok to test

@leofang
Copy link
Member Author

leofang commented Jan 16, 2025

/ok to test

@leofang
Copy link
Member Author

leofang commented Jan 16, 2025

note to self: when using this API to check the PR number
https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#list-pull-requests-associated-with-a-commit
the owner field is not the repo that the PR targets, but the repo that the PR originates from (in this case, mine). However, when the CI is triggered via the push event by the copy-pr-bot, I feel this info is lost and I can't retrieve it...

@leofang
Copy link
Member Author

leofang commented Jan 16, 2025

/ok to test

@leofang
Copy link
Member Author

leofang commented Jan 16, 2025

@leofang leofang force-pushed the doc_preview branch 2 times, most recently from d874d00 to 609caf7 Compare January 16, 2025 07:12
@leofang
Copy link
Member Author

leofang commented Jan 16, 2025

/ok to test

1 similar comment
@leofang
Copy link
Member Author

leofang commented Jan 16, 2025

/ok to test

@leofang
Copy link
Member Author

leofang commented Jan 16, 2025

/ok to test

@leofang
Copy link
Member Author

leofang commented Jan 16, 2025

/ok to test

@leofang leofang added this to the cuda-python 12-next, 11-next milestone Jan 16, 2025
@leofang
Copy link
Member Author

leofang commented Jan 16, 2025

/ok to test

@leofang leofang marked this pull request as ready for review January 16, 2025 16:09
@leofang
Copy link
Member Author

leofang commented Jan 16, 2025

(PR description updated)

ksimpson-work
ksimpson-work previously approved these changes Jan 16, 2025
@leofang
Copy link
Member Author

leofang commented Jan 16, 2025

/ok to test

rwgk
rwgk previously approved these changes Jan 16, 2025
.github/workflows/build-docs.yml Outdated Show resolved Hide resolved
.github/actions/doc_preview/action.yml Outdated Show resolved Hide resolved
@leofang
Copy link
Member Author

leofang commented Jan 16, 2025

/ok to test

@leofang
Copy link
Member Author

leofang commented Jan 16, 2025

Can anyone approve again? Let's merge and check if the cleanup happens as expected, and then we can use this in other doc PRs.

@ksimpson-work ksimpson-work self-requested a review January 16, 2025 18:17
@leofang leofang merged commit c271903 into NVIDIA:main Jan 16, 2025
69 checks passed
@leofang leofang deleted the doc_preview branch January 16, 2025 18:20
Copy link

Doc Preview CI
Preview removed because the pull request was closed or merged.

@leofang
Copy link
Member Author

leofang commented Jan 16, 2025

Verified the clean-up works too! 3db2f0c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/CD CI/CD infrastructure documentation Improvements or additions to documentation P1 Medium priority - Should do
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: Add doc preview
3 participants