feat: Maven plugin progress upload for model descriptor - #2580
feat: Maven plugin progress upload for model descriptor#2580cristianonicolai wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds upload progress feedback to the Maven plugin’s model descriptor deployment, so slow uploads and server-side processing delays no longer look like a hung build.
Changes:
- Introduces
CountingBodyPublisher+UploadProgressReporter(+ settings) to emit throttled progress lines during HTTP request upload and while awaiting the response. - Switches deploy uploads to
sendAsyncwith periodic heartbeats from the mojo thread (sendWithProgress). - Extends plugin and product documentation plus adds targeted tests for the new behavior.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/http/UploadProgressSettings.java | Adds a single configuration record for throttling/announcement thresholds with validation and defaults. |
| service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/http/UploadProgressReporter.java | Implements throttled [INFO] progress logging and final summary line formatting. |
| service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/http/CountingBodyPublisher.java | Wraps BodyPublisher to count bytes emitted to the HTTP client for progress estimation. |
| service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/DeployModelMojo.java | Routes POST/PATCH uploads through a shared helper that enables progress reporting. |
| service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/AbstractPlatformModelMojo.java | Adds sendWithProgress() to wait on sendAsync() while emitting heartbeats. |
| service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/ConfigureMojo.java | Adds an [INFO] log line indicating platform configuration fetch. |
| service/tools/maven-plugin/src/test/java/ai/timefold/solver/tools/maven/http/UploadProgressReporterTest.java | Unit tests for throttling rules and formatting without sleeping (injected clock/counter). |
| service/tools/maven-plugin/src/test/java/ai/timefold/solver/tools/maven/http/CountingBodyPublisherTest.java | Tests byte counting correctness and counter reset across re-subscription. |
| service/tools/maven-plugin/src/test/java/ai/timefold/solver/tools/maven/DeployModelMojoTest.java | Integration-style tests asserting presence/absence of progress logs and exception unwrapping behavior. |
| service/tools/maven-plugin/README.adoc | Documents how progress reporting behaves and where the implementation lives. |
| docs/src/modules/ROOT/pages/deploying-to-platform/guide.adoc | User guide note explaining when progress lines appear during deployment. |
Suppressed comments (1)
service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/http/CountingBodyPublisher.java:49
- Objects.requireNonNull should include a parameter-name message (for example, Objects.requireNonNull(subscriber, "subscriber")) so null failures are actionable and consistent with project conventions.
public void subscribe(Subscriber<? super ByteBuffer> subscriber) {
Objects.requireNonNull(subscriber);
// The client may subscribe more than once for a single logical request, for example when it resends the
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
831ffa0 to
d09e07c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (1)
service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/http/UploadProgressSettings.java:27
- UploadProgressSettings throws an IllegalArgumentException saying "must not be negative" even when maximumSilence is null, which makes misconfiguration harder to diagnose.
if (maximumSilence == null || maximumSilence.isNegative()) {
throw new IllegalArgumentException("maximumSilence must not be negative but was (" + maximumSilence + ").");
}
|
I believe the one that takes time is container image push and not the descriptor zip so not sure we really need that as it does seem like a lot of code for little value if at all. |
|
@mswiderski Indeed this is not the main issue, but didnt we see descriptor that were quite some MBs? |
yes, there are model descriptors of 3-4 mb but compared to container image size it's nothing and if someone is really on that slow network she will never reach the point where descriptor is the issue. Just saying that the value here is questionable in my opinion. |
|
@mswiderski I agree in that case, I was under the impression we already seen descriptors with almost 100Mb, thats why. Happy to drop this then. |
100MB won't fly as we have limits on the platform so that would be rejected anyway. Maybe we could actually have this limit client side too? let's not drop it yet, I just expressed single opinion so let's see others. |
winklerm
left a comment
There was a problem hiding this comment.
I agree with @mswiderski that the model descriptor upload is probably not the biggest part of the upload, so the value is a little bit questionable.
On the other hand, it might be good to see some progress, especially in case when there is an issue with the artifact repository the descriptor is uploaded to.
For that reason, I am slightly more in favor of merging this than dropping it. But let's see what the others think.
Wondering if we could get a progress report also from quarkus when it is uploading the image, but that's probably another story.
|
@winklerm I've reported it upstream to Quarkus and provided a PR, hopefully will get accepted. See quarkusio/quarkus#55958 |
d09e07c to
9adfa52
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (2)
service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/http/UploadProgressReporter.java:69
- If the underlying CountingBodyPublisher is resubscribed (for example after a redirect) and its byte counter resets to 0, the reporter can log percentages that go backwards or suppress output because lastReportedPercentage is not reset.
int percentage = (int) (transferred * 100 / contentLength);
if (percentage - lastReportedPercentage < settings.minimumPercentageDelta() && !isSilentForTooLong(now)) {
return;
}
lastReportedPercentage = percentage;
service/tools/maven-plugin/README.adoc:37
- This bullet claims a final "transfer finished" line is always logged, but the code only logs it when at least one heartbeat/waiting line was emitted (large but fast uploads can log only the up-front announcement).
- While the request is in flight the plugin reports progress. Archives of at least 1 MiB are announced up front, then at most one throttled line every 5 seconds reports the percentage uploaded, then a heartbeat reports that the upload is done and the platform is still processing, and finally a single line reports the total transferred and elapsed time. A normal deploy of an archive of a few tens of kilobytes completes before the first heartbeat and produces none of these lines. Note that `mvn -q` suppresses `[INFO]`, so it hides all progress output.
|
9adfa52 to
f59e351
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (1)
service/tools/maven-plugin/src/main/java/ai/timefold/solver/tools/maven/DeployModelMojo.java:222
- The upload request still inherits the 30-second HttpRequest timeout from configureHttpRequest(), which can cause larger uploads on slow connections to fail despite the new progress reporting.
CountingBodyPublisher bodyPublisher = new CountingBodyPublisher(BodyPublishers.ofFile(archivePath));
Builder builder = HttpRequest.newBuilder().uri(requestURI).method(method, bodyPublisher);
configureHttpRequest(builder);



Related to https://github.com/TimefoldAI/timefold-solver-enterprise/issues/807
Sample output: