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

Replace Netlify with Read the Docs build previews #103843

Merged
merged 6 commits into from
Apr 30, 2023

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Apr 25, 2023

Let's use Read the Docs for documentation previews instead of Netlify.

Benefits:

  • Most important: we can add many admins to the RtD project, compared to a single-person for Netlify which causes bottlenecks
  • We're using RtD for PEPs and devguide previews and it's working well
  • The RtD team have been very helpful, and it's nice to keep things in the Python family
  • We can use the https://github.com/readthedocs/actions/tree/main/preview action we're using on the other repos
  • As a longer term plan, I'd also like to use RtD for hosting the HTML docs rather than the build server, but that's for another discussion.

TODO

  • Set up an RtD project, needs someone with sufficient permissions to set up the webhooks. Needs to be done before merge.
  • Disable the Netlify connection to this repo. Task for Ee, can possibly be done later after merge?
  • Merge

@hugovk hugovk added docs Documentation in the Doc dir skip issue skip news labels Apr 25, 2023
Copy link
Member

@CAM-Gerlach CAM-Gerlach 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 couple comments. Also, shouldn't this be merged before turning on the RTD project so the config is correct? Or at least as soon as possible?

.github/workflows/documentation-links.yml Outdated Show resolved Hide resolved
Doc/conf.py Outdated Show resolved Hide resolved
@sobolevn
Copy link
Member

Nice idea!

@hugovk
Copy link
Member Author

hugovk commented Apr 25, 2023

Also, shouldn't this be merged before turning on the RTD project so the config is correct? Or at least as soon as possible?

The docs still build without the config, but it's slower because it builds more than just html, and also installs the other default stuff we don't need when not overriding the build commands. Iirc about 5 mins to 2 mins.

But yes, we can also merge first.

@willingc
Copy link
Contributor

Great idea. Thanks @hugovk

@hugovk hugovk marked this pull request as ready for review April 26, 2023 20:08
@@ -0,0 +1,24 @@
name: Read the Docs PR preview
Copy link
Member

Choose a reason for hiding this comment

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

Somewhat unrelated to this specific PR, but I think it would be useful to add a comment (or a docstring-like description if there is a field for it) to document what the workflow is for. If it's not possible to do it here, we could document the workflows either in a README in the dir or somewhere in the devguide.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good suggestion @ezio-melotti

Copy link
Member Author

Choose a reason for hiding this comment

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

Added!

Copy link
Member

@CAM-Gerlach CAM-Gerlach left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @hugovk !

@hugovk
Copy link
Member Author

hugovk commented Apr 30, 2023

@Mariatta has created an RtD project https://readthedocs.org/projects/cpython-previews/ and added me and @ambv as maintainers. Anyone else to add?

The first build passed!

On https://readthedocs.org/dashboard/cpython-previews/edit/ I added a "Description" and "Project homepage":

Details image

On https://readthedocs.org/dashboard/cpython-previews/advanced/ I checked "Disable Analytics", "Show version warning" and "Build pull requests for this project":

Details image

Everything else is the default.

Let's merge this PR with the config, it'll make the build times quicker.

@hugovk hugovk merged commit accb417 into python:main Apr 30, 2023
@arhadthedev
Copy link
Member

Disable the Netlify connection to this repo. Task for Ee, can possibly be done later after merge?

@ewdurbin

@hugovk
Copy link
Member Author

hugovk commented Apr 30, 2023

Pre-merge build

Build took 430 seconds with Python 3.7, built html and htmlzip:

Details image

https://readthedocs.org/projects/cpython-previews/builds/20337268/

Post-merge build with config file

Build took 247 seconds with Python 3.11, built html only:

Details image

https://readthedocs.org/projects/cpython-previews/builds/20337291/

PR deploy preview

Here's #103457 showing the preview banner at https://cpython-previews--103457.org.readthedocs.build/en/103457/

Details image

@hugovk
Copy link
Member Author

hugovk commented Apr 30, 2023

@ewdurbin Please could you disable Netlify via https://app.netlify.com/sites/python-cpython-preview/settings/deploys:

image

@ewdurbin
Copy link
Member

Done.

carljm added a commit to carljm/cpython that referenced this pull request May 1, 2023
* main: (26 commits)
  pythongh-104028: Reduce object creation while calling callback function from gc (pythongh-104030)
  pythongh-104036: Fix direct invocation of test_typing (python#104037)
  pythongh-102213: Optimize the performance of `__getattr__` (pythonGH-103761)
  pythongh-103895: Improve how invalid `Exception.__notes__` are displayed (python#103897)
  Adjust expression from `==` to `!=` in alignment with the meaning of the paragraph. (pythonGH-104021)
  pythongh-88496: Fix IDLE test hang on macOS (python#104025)
  Improve int test coverage (python#104024)
  pythongh-88773: Added teleport method to Turtle library (python#103974)
  pythongh-104015: Fix direct invocation of `test_dataclasses` (python#104017)
  pythongh-104012: Ensure test_calendar.CalendarTestCase.test_deprecation_warning consistently passes (python#104014)
  pythongh-103977: compile re expressions in platform.py only if required (python#103981)
  pythongh-98003: Inline call frames for CALL_FUNCTION_EX (pythonGH-98004)
  Replace Netlify with Read the Docs build previews (python#103843)
  Update name in acknowledgements and add mailmap (python#103696)
  pythongh-82054: allow test runner to split test_asyncio to execute in parallel by sharding. (python#103927)
  Remove non-existing tools from Sundry skiplist (python#103991)
  pythongh-103793: Defer formatting task name (python#103767)
  pythongh-87092: change assembler to use instruction sequence instead of CFG (python#103933)
  pythongh-103636: issue warning for deprecated calendar constants (python#103833)
  Various small fixes to dis docs (python#103923)
  ...
publish = "build/html"
# Do not trigger netlify builds if docs were not changed.
# Changed files should be in sync with `.github/workflows/doc.yml`
ignore = "git diff --quiet $CACHED_COMMIT_REF $COMMIT_REF . ../netlify.toml"
Copy link
Contributor

Choose a reason for hiding this comment

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

You can keep something like this on RTD (cancel the build if there is no changes at Doc/ directory, for example) by running a test command that exits with code 183. Take a look at https://docs.readthedocs.io/en/latest/builds.html#cancelling-builds for more information.

We have something like this at Read the Docs itself in our config file: https://github.com/readthedocs/readthedocs.org/blob/main/.readthedocs.yml#L28

Copy link
Member Author

Choose a reason for hiding this comment

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

Attempted in #104100.

@hugovk hugovk added the needs backport to 3.11 only security fixes label May 2, 2023
@miss-islington
Copy link
Contributor

Thanks @hugovk for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @hugovk, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker accb417c338630ac6e836a5c811a89d54a3cd1d3 3.11

hugovk added a commit to hugovk/cpython that referenced this pull request May 2, 2023
Co-authored-by: Oleg Iarygin <dralife@yandex.ru>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
(cherry picked from commit accb417)
@bedevere-bot
Copy link

GH-104083 is a backport of this pull request to the 3.11 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label May 2, 2023
hugovk added a commit that referenced this pull request May 2, 2023
…104083)

Co-authored-by: Oleg Iarygin <dralife@yandex.ru>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip issue skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.