Skip to content

Fix queue failure, refactor it and add a bunch of tests #863

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jul 1, 2020

Conversation

pietroalbini
Copy link
Member

This PR fixes the documentation builder failing to get the next crate out of the queue, refactors the whole queue handling out of DocBuilder to allow easier tests, unifies all the queue handling into the new BuildQueue and puts the number of attempts in the configuration. Along with the changes, tests were added for everything.

This PR is best reviewed commit-by-commit.
r? @jyn514

@jyn514 jyn514 added P-high High priority S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed A-builds Area: Building the documentation for a crate labels Jun 29, 2020
@pietroalbini
Copy link
Member Author

@Nemo157 addressed or responded to all review comments.

Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

Going to also run a local build to make sure things still work.

@jyn514
Copy link
Member

jyn514 commented Jun 30, 2020

Oh wait, I can't run a local build because that doesn't go through the queue 🙃 hopefully your tests exercise everything.

Can you also rebase over master so the Docker failure will go away?

@Nemo157
Copy link
Member

Nemo157 commented Jun 30, 2020

docker-compose run web queue add should work I think? (Or is the queue builder also not enabled)

@jyn514
Copy link
Member

jyn514 commented Jun 30, 2020

docker-compose run web queue add should work I think? (Or is the queue builder also not enabled)

The only way to enable the queue builder is by running the daemon, which tries to build all 300,000 crates off of crates.io. Just downloading that number of crates takes several hours and it's a real waste of bandwidth.

It would be great to refactor the builder and the daemon to be separate at some point.

@pietroalbini
Copy link
Member Author

Addressed the feedback and moved the code from src/queue.rs to src/build_queue.rs.

@jyn514 jyn514 merged commit 8631de3 into rust-lang:master Jul 1, 2020
@pietroalbini pietroalbini deleted the fix-queue branch July 1, 2020 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-builds Area: Building the documentation for a crate P-high High priority S-waiting-on-review Status: This pull request has been implemented and needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants