search

mozilla / sccache

Sccache is a ccache-like tool. It is used as a compiler wrapper and avoids compilation when possible. Sccache has the capability to utilize caching in remote storage environments, including various cloud storage options, or alternatively, in local storage.
Apache License 2.0
5.85k stars 552 forks source link

Specify server timeout #2246

Open just-an-engineer opened 3 months ago

just-an-engineer commented 3 months ago

This should fix issue #2237, where in the case a compilation is still ongoing when a shutdown request is sent, and it's a larger/complex codebase that takes longer than the default 10 seconds, the user can now specify it in their config file.

However, I'd like to also propose a breaking change for config files, moving server_startup_timeout_ms and server_shutdown_timeout_ms to a specific timing struct. I left the code in there to do this, just commented out. I'd like to make this change next time we bump up a major version. Although I can remove the comments to clean up the code until then. But what's the best practice for marking a milestone? Should I raise an issue?

just-an-engineer commented 3 months ago

If a breaking change will occur in 0.9, like @Xuanwo had mentioned in PR #2235, talking about the issue to replace rouille with axum, then I can go ahead and change this PR to include that breaking change for the config file, and we can merge this in with 0.9. If that will occur. Otherwise, it's ready to go, outside of the comment I previously wrote

sylvestre commented 2 months ago

seems that rustfmt needs to be run also, could you please document this into the readme file? thanks

codecov-commenter commented 2 months ago

Codecov Report

Attention: Patch coverage is 50.00000% with 24 lines in your changes missing coverage. Please review.

Project coverage is 51.67%. Comparing base (0cc0c62) to head (554425c). Report is 72 commits behind head on main.

Files with missing lines Patch % Lines
src/config.rs 52.50% 2 Missing and 17 partials :warning:
src/server.rs 0.00% 0 Missing and 3 partials :warning:
tests/oauth.rs 0.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2246 +/- ## =========================================== + Coverage 30.91% 51.67% +20.76% =========================================== Files 53 54 +1 Lines 20112 20751 +639 Branches 9755 9795 +40 =========================================== + Hits 6217 10724 +4507 - Misses 7922 8026 +104 + Partials 5973 2001 -3972 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

just-an-engineer commented 1 month ago

@sylvestre, could I make a flag doc or something along those lines? I feel like the readme is getting too long. Perhaps cargo doc? I haven't used it yet, though, so I'm not too sure. I just think that's a better alternative to add this option to over just the readme