bpo-37860: Add netlify deploy preview for docs #15288
Conversation
There was a problem hiding this comment.
@epicfaace Thanks for the PR!
This will definitely be much more convenient than having to manually use make html to preview rich formatting changes.
Edit: Also, as far as I can tell, the netlify preview seems to be working correctly in the PR within the author's forked repository: https://deploy-preview-5--priceless-newton-e3c22b.netlify.com.
| @@ -0,0 +1,4 @@ | |||
| # Requirements for docs build on netlify | |||
| sphinx==2.0.1 | |||
There was a problem hiding this comment.
@Mariatta Do you think that this should be pinned or float upward?
There was a problem hiding this comment.
I pinned it because that's how it was in .travis.yml.
https://github.com/python/cpython/blob/master/.travis.yml#L58
Actually, do you think it might be better to make .travis.yml to read from Doc/requirements.txt instead of specifying its own versions?
There was a problem hiding this comment.
Let's do this. Keep it pinned for now but add a comment: # Pin sphinx to version specified in .travis.yml on the line above Sphinx in requirements.txt
There was a problem hiding this comment.
Done. I think it might be better to just include python -m pip install -r Docs/requirements.txt in .travis.yml though, so that we don't have to keep two files in sync, unless there's a reason not to do so?
There was a problem hiding this comment.
@epicfaace For now, the least disruptive change was to make the modification here. Ideally, there wouldn't be duplication. Would you like to open an issue suggesting the above.
|
If it's not much of a hassle, I would appreciate it if someone could @ mention me when this PR is merged. I'll be mentioning the netlify preview in my PR for the devguide as a part of verifying rich formatting in NEWS entries. It might be worthwhile to also mention it somewhere in https://devguide.python.org/documenting/. |
@aeros167 It's probably better for you to click Notifications on the right sidebar which will subscribe you to changes in the PR 😄
Yes it would 👍 |
Haha, good point. I recently changed my notification settings to only notify me via email for @ mentions (since I was getting a bit over-flooded w/ GitHub emails), but I could probably have it different for specific PRs. I previously had it set to "Watching" for any PR that I reviewed or commented in. Also, I hadn't realized that the "Releases only" setting was an option, which notifies only for @ mentions and releases. That should do the trick. (: |
|
Since this change is involving the addition of a development tool to the repository and it wouldn't affect users of Python, I'll add a |
aeros
added
skip issue
docs
|
Thanks again for the PR @epicfaace and thanks for merging it @willingc. I'll go update my PR for the devguide. |
|
An admin now needs to actually add Netlify as an app to this repository so that it can start running and building the docs. |
I'm not certain if there's a specific person that manages the repository's apps, so I'll notify all of the admins for CPython. /cc @python/cpython-repository-admins |
|
@epicfaace have a link to the GitHub App for Netlify? |
| @@ -0,0 +1 @@ | |||
| 3.7 | |||
There was a problem hiding this comment.
Could this be specified in netlify.toml too instead of a special-purpose file?
There was a problem hiding this comment.
@brettcannon I think the easiest way to integrate it is to sign in to netlify.com with whichever account Python is using. You can use the same Netlify account that's being used with https://github.com/python/devguide. Then, you can add this repo as a "site" on netlify, and then it should ask for permission with the appropriate scopes. See https://www.netlify.com/docs/github-permissions/ |
|
@epicfaace not aware of any account, so I don't know what you're talking about. 😄 @ewdurbin do you know/control the netlify account for the org? |
|
For reference, here's the original thread where it was set up for devguide: python/devguide#463 |
|
😟 the devguide netlify was actually set up under my personal netlify account. If the PSF can create the netlify account and add me to it as a team member, I can help manage and create the netlify for CPython. |
|
I've set up an open source site on Netlify before. You need to contact Support and then they will give your account access to create a "Team". Then, you can add people to the team and create sites on it. Note that in order to qualify for this, you must have a badge that links to Netlify on the deployed site. (They suggest you add a "deploys by Netlify" badge, so it seems like this requirement would only apply to the version of the site that is deployed by Netlify, i.e. the deploy previews) https://www.netlify.com/legal/open-source-policy/ Otherwise, using a personal account on Netlify doesn't give you the ability to share permissions/management with others. |
* add netlify deploy preview * fix publish path * install python3 venv * add sudo * try without venv * install right dependencies * use python3, not python * use pip3 * python3.7 * use requirements.txt * move requirements.txt to Doc * use python 3.7 in runtime.txt * move runtime.txt * Update requirements.txt
|
I'm eager to see this feature employed. Today, I'm reviewing GH-17730, and I'd love to see the rendered output without building the toolchain or downloading the zip file and expanding it locally. |
|
@Mariatta how could we go forward with this? Could we just set this up as part of your personal netlify account (although it's not ideal, at least then the devguide and cpython repos will both be under your account, so that they could later be transferred together to a PSF account)? |
|
@epicfaace we would need @ewdurbin to set up a PSF-managed Netlify account. |
|
I've contacted Netlify to inquire about setting up a PSF team on the service. It looks like their free tier has changed and probably would not support the needs of this repo both in build minutes and collaborator count. |
|
@ewdurbin I assume you're talking about the "Pro" tier, correct (which is what Netlify provides to open-source teams for free) (1000 build minutes included/month and 3 collaborators included)? What values of build minutes / collaborator count do you think would be required for this repo? |
* add netlify deploy preview * fix publish path * install python3 venv * add sudo * try without venv * install right dependencies * use python3, not python * use pip3 * python3.7 * use requirements.txt * move requirements.txt to Doc * use python 3.7 in runtime.txt * move runtime.txt * Update requirements.txt
|
Howdy! I heard back from netlify and we have access to a Pro level account now through their Open Source Plan Policy. To get some bearings, I temporarily enabled it for this repository and opened a test pull request at #18242, which resulted in a working preview build 🎉. Overall it seems to work with that minor tweak, but the limitations of the Pro level plan will likely need to be addressed: 1000 build minutes per month I'm not sure if it's a cold cache kind of situation, but from the logs of a few test deploys (example) I noticed that cloning alone is taking over a minute per build, followed by another 5-6 minutes to install dependencies and build the docs. This means that we can currently expect to get ~170 preview builds per month before reaching our limit and beginning to incur charges of $7/500 additional build minutes. 3 concurrent builds This is a pretty active repository, so this would likely just be a headache at times. Probably not a blocker. 3 team members Probably not an issue, I'd just need to know who to add. Given those, I think we'd need to investigate some way to handle the limitations above (and figure out why the change in #18242 was necessary) before turning this on. |
|
Thanks for reaching out to netlify on this @ewdurbin! Assuming we can find a way around the monthly build time limits, this will be an awesome feature for documentation PRs; especially for larger ones that include a significant amount of markdown. For example, the "What's New" documentation often contains broken Sphinx roles (with correct syntax, but an incorrect link). These could be caught much easier with the netlify preview. One possible solution would be to use the preview selectively for documentation PRs, via a manually applied label (which would be configured to trigger a webhook). For example: In the future if we're able to increase the monthly build limit substantially (to either unlimited or high enough that we'd never reasonably reach it), we could potentially configure @bedevere-bot to automatically add the above |
|
Given the limitations of using Netlify, I've added a PR with an alternative approach using GitHub Actions in the issue (https://bugs.python.org/issue37860). Any thoughts on this approach? |
* add netlify deploy preview * fix publish path * install python3 venv * add sudo * try without venv * install right dependencies * use python3, not python * use pip3 * python3.7 * use requirements.txt * move requirements.txt to Doc * use python 3.7 in runtime.txt * move runtime.txt * Update requirements.txt
Add netlify deploy preview for docs. Once you add Netlify as an app to this github repo, then with these files added in, it should build the docs properly.
Here is an example PR with netlify enabled: epicfaace#5
And the corresponding deploy preview: https://deploy-preview-5--priceless-newton-e3c22b.netlify.com/
https://bugs.python.org/issue37860
Fixes python/core-workflow#348