admin::render_readmes: Support out of order tar files #3971
Conversation
nipunn1313
force-pushed
the
rerender_readme4
branch
2 times, most recently
from
6622730 to
40a418a
Compare
Achieves this by pulling the entire tar file into memory in the form of a hashmap. Then we can look for the files as needed. Before, we would stream the pkg response from the server and un-tar it on the fly, but this behavior is incorrect if the Cargo.toml isn't at the beginning of the archive. Could imagine this might be problematic if the entire archive won't fit in memory - but that seems unlikely? We could mitigate by limiting to certain file types, but we won't know what type the README will be. Added a test to for this case - the test fails before this change and passes with this change
nipunn1313
force-pushed
the
rerender_readme4
branch
from
40a418a to
38ce91a
Compare
|
hmm, I do admit that I'm a bit worried about pulling all of the contents into memory. we do have a few crates that are several dozens of megabytes gzipped, so uncompressing all of their contents and keeping them in memory might be problematic. |
|
One saving grace is that it's only pulling them in from memory one at a time. Also it's only doing it on the machines running the admin scripts. Several dozens of megabytes in memory seems fine? Mitigating ideas include
If I were deploying this, I'd probably try to avoid adding such heuristics/complexity until they are proven necessary. I've often found that unnecessary complexity like that can be a cure that's worse than the disease. Since it's an admin functionality, we could try it out and fix based on the issues that come up. I don't have access to run the admin functionality (of course) - but if you assigned me a task with the failing crates, I could look at them and add heuristics around the specific issues that come up. My suspicion is that nothing would come up - as it seems unlikely that we'd have gigabyte sized crates (is there a way to check easily ahead of time?) |
|
Closing based on discussion - not planning to maintain admin::render_readmes in this state. Ideally admin::render_readmes is not downloading anything from crates-io. |
Depends on #3970
(Because of community/community#4477 - github ends up rendering both diffs together. Go to the commits tab and just look at the most recent commit to review).