Publish docs as github artifacts during CI #112407
Conversation
|
(rustbot has picked a reviewer for you, use r? to override) |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment was marked as resolved.
This comment was marked as resolved.
This comment has been minimized.
This comment has been minimized.
1b1cada to
d2471bc
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
06c1b68 to
f452065
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Okay, results of the last CI run look good. @rustbot ready |
|
One question here is how much storage we expect to use for these artifacts. At 36MB/PR push, and using the last 7 days as an estimate of how many PRs we have (195) I think we can expect somewhere in the range of 10-100 GB of github storage used for this presuming PRs are pushed to somewhere between 10-100 times. Ideally I think we would limit PRs to creating a single artifact (i.e., if you push again, the old docs are replaced with new ones), which would greatly decrease the potential for accidentally using a lot of storage with frequent pushes. How feasible is that? (Storage wise we may also want to use zstd for compression. Testing locally that gets me a 20MB vs. 36MB blob.) It also looks like the artifact is a zip file of a tarball. Is that indirection necessary? Can we avoid the zip archive? I think another aspect is discoverability. Right now digging up the artifact is pretty painful - there's no UI surface area for it that I can see. Maybe we can have a custom check in triagebot or elsewhere that adds a dedicated clickable check to PR CI that links to a rendered copy of those docs somewhere? Seems like that ought to be feasible... Why are we naming the artifact with a SHA hash? Is that trying to avoid some kind of overwriting? |
|
It looks like there are some references in various github docs about "github pages preview" -- I'll see if we can talk with GitHub about maybe using that for this. |
|
@rustbot author |
|
For rendering, I was thinking of having a site, something like When that page is first opened, it pulls the artifact from actions and extracts the contents. Then it will just serve the docs normally. It's on demand so there's no wasted space or energy if nobody uses it for a given PR. After a certain time period of no requests, the docs for a given PR will be deleted. I suppose we could just do the whole thing by checking out with git and running rustdoc on the files ourselves, but then we'd have to worry about sandboxing and the rustdoc run would be extra work. I do wonder if there's a heuristic we can use to avoid uploading a docs artifact when no docs changes happened (or maybe even when the changes are trivial). I think I may have mentioned before - we could choose to only upload the parts of docs that were changed. |
This does sound reasonable but I have looked and don't know of any way to do this. I think you could expire them manually but it would be nice if there is an automated way (maybe a nice thing to ask Github?)
I've updated the PR to use zstd (L12 gets it down further to 14MB) and 10 -> The double compression thing is really goofy, but there doesn't seem to be a way around it actions/upload-artifact#39
Agreed, I am curious what they suggest. I think as-is could be a MVP if those other things take time, but having some way to directly render would be best.
Just to be able to identify the commit if you download >1, I don't believe the artifacts overwrite each other at all. |
8e046bd to
e1184d4
Compare
That is kind of what I originally had in mind. If that site existed, we could just upload the files directly to S3 or something via actions so the site can be "dumb" (not sure how API keys would work here though). I'm not opposed to doing something like this if it's desirable, but I need to know what would be possible and where exactly to go. |
|
Looks like viewing is a well-known request actions/upload-artifact#14 |
This PR saves library docs as github artifacts so they can be easily viewed for review. Discussed in <https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/Building.20docs.20for.20PR.20CI>
e1184d4 to
696b0dd
Compare
|
@Mark-Simulacrum do you consider this specific PR blocked until the conversation with github (I believe you said 3 weeksish, but I don't really expect them to have any solutions) and/or us setting up an infra-based viewing solution like @pitaj suggested? Or would it be suitable to merge this as-is and improve the UX later? (for which I would open an issue) I decreased the retention time further to 5 days. The compression and retention reduction should drop storage costs to 27% of the values you estimated in #112407 (comment) (not sure how exactly you did the estimate but based on percentages alone, usage should drop from 10 |
|
No, I don't think so. I was going to wait when that conversation was today (as originally planned, we had to move it out), but I think we'll likely move ahead with this PR before then - I just need to sit down and review it. My hope is to do that before the weekend, but if not that's when I do a pass over all PRs open + marked as ready for review. |
|
Thanks for clarifying - I wasn't intending to pressure a review, just curious if the labels were accurate or if I should direct my effort somewhere |
|
@bors r+ Let's go ahead with this as-is, and we can come back and update further if there's any problems encountered in practice. |
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (1d7d824): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 656.882s -> 656.42s (-0.07%) |
Discussed here: https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/Building.20docs.20for.20PR.20CI
The goal is to make docs available for download after CI runs on PRs, for easy review of API changes.
Notes:
core,alloc,std,test,proc_macro)ci-artifacts. No way to customize this based on contained files unfortunately [feat req] Make file name the default actions/upload-artifact#349You can find the results at the bottom of the CI "summary" page: