Skip to content

Conversation

@HassanAbouelela
Copy link
Member

Updates the build the script to use the new API provided by the site to fetch and download artifacts.

Updates the build the script to use the new API provided by the site to
fetch and download artifacts.

Signed-off-by: Hassan Abouelela <hassan@hassanamr.com>
@HassanAbouelela HassanAbouelela added a: documentation Pull requests which update documentation t: enhancement labels Aug 12, 2022
@netlify
Copy link

netlify bot commented Aug 12, 2022

Deploy Preview for bot-core ready!

Name Link
🔨 Latest commit cd1b9b8
🔍 Latest deploy log https://app.netlify.com/sites/bot-core/deploys/6358f93c158f6900081d4016
😎 Deploy Preview https://deploy-preview-122--bot-core.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@HassanAbouelela
Copy link
Member Author

This is failing because the multiversion build takes more than 3 minutes currently. There are a few options:

  1. Limit the number of versions built. This is possible, but making it dynamic is hard since we can only specify a regex to match each tag name against. One way to make it work is to grab all tag names ourselves and combine all the allowed ones in a simple regex. Besides technical challenges, it's also an arbitrary limitation that reduces the benefit of having access to historical docs versions, which was the initial intention with this system.
  2. Use the latest version build in the deployment, instead of the multiversion. This runs a lot quicker since it's just building one version, and wouldn't require much effort, just changing a few environment variables in netlify
  3. Extend the API deadline. This is not really a solution, and the issue will return in the future if the build takes more than the new allotted deadline.
  4. Remove the deadline entirely.
  5. Don't make the action mandatory/don't run on PRs. This is not ideal since the action can sometimes be very fragile, and it's nice to know what broke it easily.
  6. A combination of 1 and 5 (sort of). We could limit the number of versions built in PRs to something more reasonable such as 10, then do the full build on actual deployments. This has the same downsides as both solutions though, since it can make it harder to spot errors, and still requires work to make dynamic.

@HassanAbouelela
Copy link
Member Author

cc @ChrisLovering if you have any thoughts

@ChrisLovering
Copy link
Member

I like the idea of not building multi-version for PRs. Having long CI in PRs is annoying anyway, so only building the new stuff sounds good.

@ChrisLovering
Copy link
Member

Maybe we could even go as far as storing/caching the "old" versions of sphinx-mutliversion, and pulling form that, rather than building each time.

That's likely something we can look into in the future though, as it doesn't sound trivial.

We'd also still want a way toe force re-build all old versions too, in case we changing theming or something.

@HassanAbouelela
Copy link
Member Author

HassanAbouelela commented Aug 12, 2022

It looks like none of the solutions I proposed will actually work since both doc builds are under the same action, so it will still be marked pending, even once the latest version is built. If we disable the multiversion build in PRs, that'll resolve the issue in PRs, but will still fail on the main branch. I'll just bump the site deadline for now, and hope we find a better solution.

Signed-off-by: Hassan Abouelela <hassan@hassanamr.com>
@HassanAbouelela
Copy link
Member Author

HassanAbouelela commented Aug 12, 2022

Well, I've stumbled upon an interesting discovery. Each build contains metadata used to speed-up rebuilds, but it contains a bunch of binary data that increases the build size. Deleting those files reduces the size from 75 MB to 22MB, which had the added benefit of reducing the upload time from 1m19s to 40s (total 2:51 to 2:11). There's further possible gain from removing the duplicated _static folder (~700Kb/version), but I'm not sure how that'll work between different versions, and it would require rewriting all the HTML to point to the root instead of the one it was using, so I decided not to pursue that for now.

I think I'll leave that change in, though it's more of a QoL thing than a permanent fix.

@HassanAbouelela HassanAbouelela merged commit 6ae7868 into main Oct 26, 2022
@HassanAbouelela HassanAbouelela deleted the update-static branch October 26, 2022 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: documentation Pull requests which update documentation t: enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants