jetty / jetty.project

Eclipse Jetty® - Web Container & Clients - supports HTTP/2, HTTP/1.1, HTTP/1.0, websocket, servlets, and more
https://eclipse.dev/jetty
Other
3.84k stars 1.91k forks source link

Issue #929 implement a utility class to save large downloads to a file #12292

Closed arsenalzp closed 9 hours ago

arsenalzp commented 1 week ago

Hello, This PR is trying to implement feature described in the issue #929.

Fixes: #929

joakime commented 1 week ago

The CI build failed due to javadoc issues.

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-javadoc-plugin:3.7.0:javadoc-no-fork (default) on project jetty-client: An error has occurred in Javadoc report generation: 
[ERROR] Exit code: 1
[ERROR] /home/jenkins/agent/workspace/jetty.project_PR-12292/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/PathResponseListener.java:40: error: reference not found
[ERROR]  * Implementation of {@link Listener} that produces an {@link Path}
[ERROR]                             ^
[ERROR] /home/jenkins/agent/workspace/jetty.project_PR-12292/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/PathResponseListener.java:42: error: unknown tag: URL
[ERROR]  * like curl <URL> -o file.bin does.
[ERROR]              ^
[ERROR] /home/jenkins/agent/workspace/jetty.project_PR-12292/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/PathResponseListener.java:50: error: unknown tag: Path
[ERROR]  *  CompletableFuture<Path> completable = PathResponseListener.write(request, Path.of("/tmp/file.bin"));
[ERROR]                      ^
[ERROR] /home/jenkins/agent/workspace/jetty.project_PR-12292/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/PathResponseListener.java:52: error: unexpected end tag: </p>
[ERROR]  * </p>
[ERROR]    ^
[ERROR] 4 errors

You can see your CI builds here -> https://jenkins.webtide.net/blue/organizations/jenkins/jetty.project/activity?branch=PR-12292

arsenalzp commented 1 week ago

The CI build failed due to javadoc issues.

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-javadoc-plugin:3.7.0:javadoc-no-fork (default) on project jetty-client: An error has occurred in Javadoc report generation: 
[ERROR] Exit code: 1
[ERROR] /home/jenkins/agent/workspace/jetty.project_PR-12292/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/PathResponseListener.java:40: error: reference not found
[ERROR]  * Implementation of {@link Listener} that produces an {@link Path}
[ERROR]                             ^
[ERROR] /home/jenkins/agent/workspace/jetty.project_PR-12292/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/PathResponseListener.java:42: error: unknown tag: URL
[ERROR]  * like curl <URL> -o file.bin does.
[ERROR]              ^
[ERROR] /home/jenkins/agent/workspace/jetty.project_PR-12292/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/PathResponseListener.java:50: error: unknown tag: Path
[ERROR]  *  CompletableFuture<Path> completable = PathResponseListener.write(request, Path.of("/tmp/file.bin"));
[ERROR]                      ^
[ERROR] /home/jenkins/agent/workspace/jetty.project_PR-12292/jetty-core/jetty-client/src/main/java/org/eclipse/jetty/client/PathResponseListener.java:52: error: unexpected end tag: </p>
[ERROR]  * </p>
[ERROR]    ^
[ERROR] 4 errors

You can see your CI builds here -> https://jenkins.webtide.net/blue/organizations/jenkins/jetty.project/activity?branch=PR-12292

Hello, Thank you for your advice, fixed.

arsenalzp commented 1 week ago

All tests passed on my laptop with JDK17. Should I also run tests with JDK22?

sbordet commented 1 week ago

All tests passed on my laptop with JDK17. Should I also run tests with JDK22?

Our CI environment will do it, but sure if you want to try, give it a go.

sbordet commented 3 days ago

@arsenalzp, did you have time to review my comments? Thanks!

arsenalzp commented 3 days ago

@arsenalzp, did you have time to review my comments? Thanks!

Hello, Sure, I did, it is very appreciated. I'm working on it!

arsenalzp commented 2 days ago

It is failed on JDK21, previous failure was on JDK22. On my laptop tests passed on JDK17 and JDK22.

arsenalzp commented 1 day ago

Good work @arsenalzp -- let's finish with the last touches.

I would like to tackle with another issue, something like the current one. Is it still possible?

sbordet commented 1 day ago

I would like to tackle with another issue, something like the current one. Is it still possible?

It is always possible to tackle issues 😃 Release 12.0.14 is however about to be closed, so the other issue will be scheduled in the next release.

arsenalzp commented 1 day ago

I would like to tackle with another issue, something like the current one. Is it still possible?

It is always possible to tackle issues 😃 Release 12.0.14 is however about to be closed, so the other issue will be scheduled in the next release.

So, I would like to take something for my level, and interesting like this particular Issue.

sbordet commented 1 day ago

So, I would like to take something for my level, and interesting like this particular Issue.

Fine, but first let's finish this one 😄

You can pick any that you like that has the label Help-Wanted.

sbordet commented 9 hours ago

@arsenalzp please have a look at the final result. I have simplified a few things with respect to your work, and if you have questions ask away. Thanks for this contribution, it's merged.

arsenalzp commented 5 hours ago

Everything is fine, thank you for your support!