Store build logs in S3 #1214
Store build logs in S3 #1214
Conversation
jyn514
added
A-builds
src/test/fakes.rs
Outdated
| assert!( | ||
| !successful, | ||
| "default is success, can only override to failure" | ||
| ); | ||
| assert!( | ||
| self.builds.is_empty(), | ||
| "cannot use custom builds with build_result_successful" | ||
| ); |
There was a problem hiding this comment.
This is a good idea, we should add more assertions like this :) I'll work on that.
In the meantime though, since the boolean has to be false, can you statically enforce that by not taking a parameter and changing the name to build_result_failed?
|
|
||
| let value: serde_json::Value = serde_json::from_str( | ||
| &env.frontend() | ||
| .get("/crate/foo/0.1.0/builds.json") |
There was a problem hiding this comment.
This seems like a misfeature. Is anyone actually using this?
@pietroalbini what do you think about just removing this? It seems silly to have exactly one page that has JSON available, especially since you need to know the version ahead of time.
There was a problem hiding this comment.
That endpoint is used by crates.io.
We should decide if we want to support running docs.rs in prod without the database (we shouldn't be special casing build logs when documentation can still be stored in the DB). Since we have min.io for testing locally and all our data in prod is in S3, I guess there's no real reason to keep the DB support around? @pietroalbini what do you think? |
Ugh, I'll review #1216. Do you know why it only failed here? It's been working fine on other PRs ... |
Well, I say S3, but I actually mean "the configured storage", so in tests they're still stored in the DB, just in the files table instead. |
3 new web tests may have just put it over the limit. |
This doesn't use S3 directly, it uses |
It's not something that's inserted into the database with the rest of the result.
This is a first step on the path to #787
The first 4 commits are refactoring, adding tests for the builds list and build logs and splitting them into separate handlers and templates (I don't think there's much benefit to having the list of other builds below the log, and it makes the code simpler to reason about).
The last commit adds support for rendering the build log from either database or S3, and changes the build process to upload to S3.
The next step would be to migrate existing build logs from the database to S3 so we can delete the code checking for both. Then thread the build logs for other targets through and upload and render them all.
We could also consider bumping the max build log size up, which would allow reverting #1192 if having verbose logs would be useful.