GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Replace Netlify with Read the Docs build previews by hugovk · Pull Request #103843 · python/cpython

Conversation

@hugovk

@hugovk hugovk commented Apr 25, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
arhadthedev and CAM-Gerlach reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
@hugovk
Replace Netlify with Read the Docs build previews
a4f030b
@hugovk hugovk added docs Documentation in the Doc dir skip issue skip news labels Apr 25, 2023
arhadthedev
arhadthedev reviewed Apr 25, 2023
View reviewed changes
Comment thread .github/workflows/documentation-links.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.
CAM-Gerlach
CAM-Gerlach reviewed Apr 25, 2023
View reviewed changes

@CAM-Gerlach CAM-Gerlach left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread .github/workflows/documentation-links.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/conf.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
sobolevn
sobolevn reviewed Apr 25, 2023
View reviewed changes
Comment thread .github/workflows/documentation-links.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.
@sobolevn

sobolevn commented Apr 25, 2023

Copy link
Copy Markdown
Member

Nice idea!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
CAM-Gerlach
CAM-Gerlach reviewed Apr 25, 2023
View reviewed changes
Comment thread .github/workflows/documentation-links.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.
@hugovk

hugovk commented Apr 25, 2023

Copy link
Copy Markdown
Member Author

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.
CAM-Gerlach reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@willingc

willingc commented Apr 26, 2023

Copy link
Copy Markdown
Contributor

Great idea. Thanks @hugovk
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
hugovk and others added 4 commits April 26, 2023 20:09
@hugovk
Merge remote-tracking branch 'upstream/main' into rtd-docs-preview
855290c
@hugovk @arhadthedev @CAM-Gerlach
Apply suggestions from code review
8008308 
Co-authored-by: Oleg Iarygin <[email protected]>
Co-authored-by: C.A.M. Gerlach <[email protected]>
@hugovk
Cancel in-progress workflows
c421383
@hugovk
Set is_deployment_preview flag for PR builds instead of all RtD builds
2d04997
@hugovk
hugovk marked this pull request as ready for review April 26, 2023 20:08
@hugovk
hugovk requested a review from ezio-melotti as a code owner April 26, 2023 20:08
ezio-melotti
ezio-melotti reviewed Apr 26, 2023
View reviewed changes
Comment thread .github/workflows/documentation-links.yml
@@ -0,0 +1,24 @@
name: Read the Docs PR preview

@ezio-melotti ezio-melotti Apr 26, 2023

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@willingc willingc Apr 26, 2023

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Good suggestion @ezio-melotti

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@hugovk hugovk Apr 27, 2023

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Added!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@hugovk
Add a comment describing the purpose of the workflow
8d7a571
CAM-Gerlach
CAM-Gerlach approved these changes Apr 27, 2023
View reviewed changes

@CAM-Gerlach CAM-Gerlach left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM, thanks @hugovk !

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added the awaiting merge label Apr 27, 2023
@CAM-Gerlach CAM-Gerlach added the DO-NOT-MERGE label Apr 27, 2023
@hugovk hugovk mentioned this pull request Apr 29, 2023
Netlify preview builds silently fail with Deploy Preview canceled #103604
Closed
@arhadthedev arhadthedev mentioned this pull request Apr 29, 2023
gh-103604: Fix incorrect paths in netlify.toml #103605
Closed
AA-Turner
AA-Turner approved these changes Apr 29, 2023
View reviewed changes
@hugovk hugovk removed the DO-NOT-MERGE label Apr 30, 2023
@hugovk

hugovk commented Apr 30, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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

  • https://readthedocs.org/projects/cpython-previews/builds/20337232/
  • https://cpython-previews.readthedocs.io/en/latest/

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hugovk
hugovk merged commit accb417 into python:main Apr 30, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Apr 30, 2023
@arhadthedev

arhadthedev commented Apr 30, 2023

Copy link
Copy Markdown
Member

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

@ewdurbin
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hugovk

hugovk commented Apr 30, 2023

Copy link
Copy Markdown
Member Author

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hugovk

hugovk commented Apr 30, 2023

Copy link
Copy Markdown
Member Author

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

image
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@ewdurbin

ewdurbin commented Apr 30, 2023

Copy link
Copy Markdown
Member

Done.
hugovk and arhadthedev reacted with rocket emoji
All reactions
  • 🚀 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
carljm added a commit to carljm/cpython that referenced this pull request May 1, 2023
@carljm
Merge branch 'main' into supermon
159b2aa 
* 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)
  ...
humitos
humitos reviewed May 1, 2023
View reviewed changes
Comment thread netlify.toml
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"

@humitos humitos May 1, 2023

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
hugovk reacted with eyes emoji
All reactions
  • 👀 1 reaction

@hugovk hugovk May 3, 2023

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Attempted in #104100.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@hugovk hugovk mentioned this pull request May 2, 2023
Docs: Fix build preview banner #104082
Closed
@hugovk hugovk added the needs backport to 3.11 only security fixes label May 2, 2023
@miss-islington

miss-islington commented May 2, 2023

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington

miss-islington commented May 2, 2023

Copy link
Copy Markdown
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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington miss-islington assigned hugovk May 2, 2023
hugovk added a commit to hugovk/cpython that referenced this pull request May 2, 2023
@hugovk @arhadthedev @CAM-Gerlach
Replace Netlify with Read the Docs build previews (python#103843)
a821a18 
Co-authored-by: Oleg Iarygin <[email protected]>
Co-authored-by: C.A.M. Gerlach <[email protected]>
(cherry picked from commit accb417)
@bedevere-bot

bedevere-bot commented May 2, 2023

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@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
@hugovk @arhadthedev @CAM-Gerlach
[3.11] Replace Netlify with Read the Docs build previews (#103843) (#…
b07bae6 
…104083)

Co-authored-by: Oleg Iarygin <[email protected]>
Co-authored-by: C.A.M. Gerlach <[email protected]>
@hugovk hugovk mentioned this pull request May 2, 2023
RtD docs previews: Cancel building PRs if no changes in Doc dir #104100
Merged
@jiridanek jiridanek mentioned this pull request Jan 2, 2024
Possible heap-use-after-free in ctypes in Python 3.12 #113576
Closed
@KowalskiThomas KowalskiThomas mentioned this pull request Mar 3, 2026
Use reusable-docs.yml in documentation-links #145346
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@willingc willingc willingc left review comments
@sobolevn sobolevn sobolevn left review comments
@arhadthedev arhadthedev arhadthedev left review comments
@ezio-melotti ezio-melotti ezio-melotti left review comments
@AA-Turner AA-Turner AA-Turner approved these changes
@CAM-Gerlach CAM-Gerlach CAM-Gerlach approved these changes
+1 more reviewer
@humitos humitos humitos left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@hugovk hugovk

Labels

docs Documentation in the Doc dir skip issue skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

11 participants

@hugovk @sobolevn @willingc @arhadthedev @ewdurbin @miss-islington @bedevere-bot @humitos @AA-Turner @CAM-Gerlach @ezio-melotti
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.