-
Notifications
You must be signed in to change notification settings - Fork 950
[1.x] Allow users to configure the timeout when publishing to the Maven Central repo #8171
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
Conversation
@@ -66,7 +69,6 @@ class SonaClient(reqTransform: Request => Request) extends AutoCloseable { | |||
FormPart("bundle", bundleZipPath.toFile()) | |||
) | |||
) | |||
.withRequestTimeout(600.second) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already configured in http
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current implementation uses 2 minute timeout for status check type of request, and 10 minutes for the bundle upload. It seems like you are collapsing the two into one setting but with 2 minute timeout here. I feel like it might be better to pick a large enough value for upload timeout (60 min?) rather than make this configurable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Current implementation uses 2 minute timeout for status check type of request, and 10 minutes for the bundle upload. It seems like you are collapsing the two into one setting but with 2 minute timeout here
My bad. It's fixed in my latest changes
I feel like it might be better to pick a large enough value for upload timeout (60 min?) rather than make this configurable.
I'm just scared that if it's not enough for someone, we'll need to make a new change and publish a new version of sbt, which can delay people's work. I think it'd be simpler to make it configurable so it can fit any need, like it was in sbt-sonatype
6428e83
to
09e55cc
Compare
…tes` and should only be used for upload
09e55cc
to
8c0010a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall LGTM, but for future reference, please split out non-semantic-change refactorings out to a separate pull request in the future for easier code review.
Thanks @eed3si9n. Would it be possible for you to release a new version with this change, please? |
https://eed3si9n.com/sbt-1.11.3 is released |
@eed3si9n I made this change in zio-aws: https://github.com/zio/zio-aws/pull/1554/files#diff-928a6eabff1094f1b64a2d1b6bc7574c5a483476ec3fee98c16d1fe46aae9d0dR104 java.util.concurrent.TimeoutException: Futures timed out after [1215 seconds]
at scala.concurrent.impl.Promise$DefaultPromise.ready(Promise.scala:269)
at scala.concurrent.impl.Promise$DefaultPromise.result(Promise.scala:273)
at scala.concurrent.Await$.$anonfun$result$1(package.scala:223)
at scala.concurrent.BlockContext$DefaultBlockContext$.blockOn(BlockContext.scala:57)
at scala.concurrent.Await$.result(package.scala:146)
at sbt.internal.sona.SonaClient.awaitWithMessage(Sona.scala:174)
at sbt.internal.sona.SonaClient.uploadBundle(Sona.scala:86)
at sbt.internal.sona.Sona.uploadBundle(Sona.scala:35)
at sbt.internal.librarymanagement.Publishing$.sonatypeReleaseAction(Publishing.scala:57)
at sbt.internal.librarymanagement.Publishing$.$anonfun$sonaRelease$1(Publishing.scala:24) (See https://github.com/zio/zio-aws/actions/runs/16110990896/job/45458269962) Would you have any why this didn't respect the |
I should've caught this during the code review but it seems like this PR added the setting at the project level, instead of the global level. So to override the value you have to rewire it at the root project level, instead of ThisBuild. |
deploymentStatusF(deploymentId) | ||
} | ||
Await.result(res, 600.seconds) | ||
Await.result(res, 10.minutes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also there's still 10.minutes hardcoded here.
The publishing of zio-aws takes a lot of time because it publishes a lot of libs and because of that fails to publish to the new Maven Central repo as the publishing process timeouts.
See https://github.com/zio/zio-aws/actions/runs/16018360517/job/45195274956
Previously, with the sbt-sonatype plugin, we were able to configure the
sonatypeTimeoutMillis
setting(https://github.com/zio/zio-aws/pull/1517/files#diff-928a6eabff1094f1b64a2d1b6bc7574c5a483476ec3fee98c16d1fe46aae9d0dL101)