Skip to content

JDK Transport: Do no longer leverage temp file for transfering artifact#1755

Merged
kwin merged 3 commits intomasterfrom
feature/jdk-transport-prevent-temp-file-for-put
Jan 31, 2026
Merged

JDK Transport: Do no longer leverage temp file for transfering artifact#1755
kwin merged 3 commits intomasterfrom
feature/jdk-transport-prevent-temp-file-for-put

Conversation

@kwin
Copy link
Copy Markdown
Member

@kwin kwin commented Jan 16, 2026

with PUT

Directly transfer based on the PutTask's InputStream. Improve retry handling to not retry (some) BodyPublisher exceptions.

Following this checklist to help us incorporate your
contribution quickly and easily:

  • Your pull request should address just one issue, without pulling in other changes.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body.
    Note that commits might be squashed by a maintainer on merge.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied.
    This may not always be possible but is a best-practice.
  • Run mvn verify to make sure basic checks pass.
    A more thorough check will be performed on your pull request automatically.
  • You have run the integration tests successfully (mvn -Prun-its verify).

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

@kwin kwin requested a review from cstamas January 16, 2026 17:36
@kwin kwin force-pushed the feature/jdk-transport-prevent-temp-file-for-put branch 2 times, most recently from 3ed3e8c to 5b011e1 Compare January 18, 2026 18:44
@cstamas
Copy link
Copy Markdown
Member

cstamas commented Jan 27, 2026

Where are we with this?

@kwin
Copy link
Copy Markdown
Member Author

kwin commented Jan 27, 2026

I need to relax some more ITs

@kwin kwin force-pushed the feature/jdk-transport-prevent-temp-file-for-put branch 3 times, most recently from ae65795 to 4c7d694 Compare January 27, 2026 17:20
@kwin kwin marked this pull request as ready for review January 27, 2026 17:30
}
notifyProgress(b, 0, numBytesRead);
}
return super.read(b);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

? is invoked 2 times, no?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, gonna fix this

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in dce23de.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but one thing bothers me: how did double invocation slipped thru UT? Why did not it fail? We obviously don't test this path? Or we don't validate what we got?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually only the read with 3 arguments is used (e.g. when copying to an output stream). Would require dedicated unit tests to cover this

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a unit test with a little help from Claude.

@kwin kwin requested a review from cstamas January 29, 2026 16:44
kwin added 3 commits January 30, 2026 15:12
with PUT

Directly transfer based on the PutTask's InputStream.
Improve retry handling to not retry (some) BodyPublisher exceptions.
@kwin kwin force-pushed the feature/jdk-transport-prevent-temp-file-for-put branch from dce23de to 9d141ef Compare January 30, 2026 14:13
@kwin kwin added the enhancement New feature or request label Jan 30, 2026
@kwin kwin merged commit d13e45f into master Jan 31, 2026
20 checks passed
@kwin kwin deleted the feature/jdk-transport-prevent-temp-file-for-put branch January 31, 2026 09:15
@github-actions github-actions bot added this to the 2.0.15 milestone Jan 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants