| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
I ran into the same ArithmeticException, so I checked both open fixes for #11416 locally against 2ac3c9773 on JDK 21.0.9. Here is what I measured. The fix and the test hold up. With only the new LoggedPullImageResultCallbackTest.java applied and the main source reverted to main, the test fails with exactly the reported error: java.lang.ArithmeticException: / by zero
at org.testcontainers.images.LoggedPullImageResultCallback.onComplete(LoggedPullImageResultCallback.java:120)
With the full PR, it passes. So the test pins the change rather than passing either way. Worth noting the reproduction is 0 / 0 (the response item carries no progressDetail, so downloadedLayerSize() is 0), which is the same division as the real sub-second case. Spotless will fail CI. ./gradlew :testcontainers:spotlessJavaCheck fails on the new test file: line 21 is 129 characters against the repo's printWidth: 120 (gradle/spotless.gradle). spotlessApply rewrites it to: PullResponseItem item = mapper.readValue(
"{\"status\":\"Download complete\",\"id\":\"layer1\"}",
PullResponseItem.class
);The start != null half of the diff is not exercised. I reverted just that hunk and kept Math.max(1, duration), and the new test still passes; Math.max alone is what fixes #11416. Is there a path where onComplete() runs without onStart()? Both call sites (RemoteDockerImage#pullImage and DockerClientFactory) go through pullImageCmd.exec(...) and I could not construct one. If there is none, dropping the ternary keeps this to a one-line change; if there is, it is a different bug from the one in the issue and would be worth a line in the description. One thing worth flagging before this gets a decision. #11417 fixes the same issue and is approved, but it does not work. It computes Duration.between(start, Instant.now()).get(ChronoUnit.MILLIS) / 1000.0, and Duration.get(TemporalUnit) supports only SECONDS and NANOS. On JDK 21.0.9: Duration.ofMillis(1500).getSeconds() -> 1 Duration.ofMillis(1500).get(ChronoUnit.MILLIS) -> java.time.temporal.UnsupportedTemporalTypeException: Unsupported unit: Millis Duration.ofMillis(1500).toMillis() -> 1500 I applied #11417 onto 2ac3c9773 and ran this PR's test against it. It compiles and then fails with UnsupportedTemporalTypeException: Unsupported unit: Millis. Since that accessor throws for any duration, it would throw on every completed pull, not just sub-second ones. Optional. If the intent behind #11417 (a real rate for sub-second pulls) is wanted, toMillis() is the supported accessor and keeps integer arithmetic: final long millis = start != null ? Duration.between(start, Instant.now()).toMillis() : 0;
...
FileUtils.byteCountToDisplaySize(downloadedLayerSize * 1000 / Math.max(1, millis))That also removes the small inconsistency this PR leaves behind, where a sub-second pull logs pulled in 0s (downloaded 50 MB at 50 MB/s) because the duration in the message and the duration used for the rate are different numbers. downloadedLayerSize * 1000 stays well inside long for any realistic image size. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Fixes #11416
Description
When pulling an image completes in less than 1 second (sub-second duration), Duration.between(start, Instant.now()).getSeconds() returns 0.
Calculating transfer speed as downloadedLayerSize / duration throws java.lang.ArithmeticException: / by zero.
This PR uses Math.max(1, duration) when calculating transfer speed in LoggedPullImageResultCallback#onComplete(), preventing ArithmeticException.
Changes