add small thread sleep between publishes - for #224 #247
Conversation
| @@ -702,6 +702,9 @@ fn release_packages<'m>( | |||
|
|
|||
| if pkg.config.registry().is_none() { | |||
| cargo::wait_for_publish(crate_name, &base.version_string, timeout, dry_run)?; | |||
There was a problem hiding this comment.
(it's probably possible to write a more advanced solution with a curl to crates.io to check that the previous upload has completed, but I'm not even sure that's perfect if all that's happening under the hood is that the write operation (the publish) propagates to all of crates' replicas.)
In theory, that is what wait_for_publish is doing but for some reason, it isn't. Looking at people's logs in #224, we are waiting, for example
[2020-09-10T07:57:34Z INFO ] Waiting for publish to complete...
[2020-09-10T07:57:40Z INFO ] Running cargo publish on kube
Here we waited for 6s. Yet somehow, it wasn't good enough. Apparently, the part of the index we are checking gets updated but not everything needed to publishing. We are then racing for that to be updated.
There was a problem hiding this comment.
Yeah, I am not sure. AFAIKT from the documentation in https://docs.rs/crates-index/0.16.2/crates_index/struct.Index.html you are doing the right thing in wait_for_publish.
src/main.rs
Outdated
| // give crates a little leeway to propagate a successful publish | ||
| // this should avoid failed to select failures where the next pkg in line depends on the previous | ||
| std::time::Duration::from_secs(5); |
There was a problem hiding this comment.
Maybe update the comment, based on my above analysis
// HACK: Even once the index is updated, there seems to be another step before the publish is fully ready. We don't have a way yet to check for that, so waiting for now in hopes everything is ready
Also, should this be in wait_for_publish? On one hand, it keeps all related logic together. On the other hand, its helpful to have hacks stand out.
There was a problem hiding this comment.
Have updated the comment. I think the wait_for_publish is doing the right thing, so maybe it just takes some times for writes to crates.io to propagate to all its read-replicas (or something like that). At any rate, it's only a problem when publishing more than one crate, so probably good to keep the hack here.
There was a problem hiding this comment.
The insiduous thing about race conditions is you can never know if your change actually helped because you might have just not hit the race when testing your fix. Or the problem might go away and you don't need the hack anymore
:(
|
It looks like I am hitting the issue of not waiting enough before one of the newly published crates is available before proceeding to the next one. Makes it much harder to resume the process. |
|
Maybe it would make sense to allow continuing if trying to re-upload a crate with the same version? |
Hey all. Thank you for cargo-release. It's very helpful!
This particular issue (#224) keeps biting me, and while this PR has only a dumb/ugly solution, I do think it should fix the problem. If the chosen time is bad, we could parametrise the second value into argv.
(it's probably possible to write a more advanced solution with a curl to crates.io to check that the previous upload has completed, but I'm not even sure that's perfect if all that's happening under the hood is that the write operation (the publish) propagates to all of crates' replicas.)