Skip to content
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

Support large file uploads with Reactive REST Client #24438

Merged
merged 1 commit into from Mar 21, 2022

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Mar 21, 2022

Fixes: #24405

@geoand
Copy link
Contributor Author

geoand commented Mar 21, 2022

The reason I have not added a test for this is because to actually have a test for the problem mentioned in #24402, we would need to download a 2GB file in CI, which could be problematic for a number of reasons...

@geoand geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Mar 21, 2022
@geoand geoand merged commit 6271099 into quarkusio:main Mar 21, 2022
@quarkus-bot quarkus-bot bot added this to the 2.8 - main milestone Mar 21, 2022
@geoand geoand deleted the #24405 branch March 21, 2022 17:22
@quarkus-bot quarkus-bot bot added kind/bugfix and removed triage/waiting-for-ci Ready to merge when CI successfully finishes labels Mar 21, 2022
@gsmet
Copy link
Member

gsmet commented Apr 28, 2022

@geoand hey. So we wanted to backport this one to 2.7 with Rostislav as you requested but it doesn't apply cleanly and I'm not confident enough to solve the conflict myself and hope for the best. Could you have a look at it if you have time and prepare a specific backport?

Maybe wait for our first batch of 2.7.6 backport to get in the 2.7 branch as it touches files that were updated by some other PRs we already backported.
I'll let you know when it's done.

@gsmet
Copy link
Member

gsmet commented Apr 28, 2022

@geoand btw, let's be cautious here: if we end up having to write a brand new patch, it might not be a good idea to backport it given it's not tested so let's assert and rediscuss if needed.

/cc @rsvoboda

@geoand
Copy link
Contributor Author

geoand commented Apr 28, 2022

I'll let you know when it's done.

Sure, no problem

@gsmet
Copy link
Member

gsmet commented May 12, 2022

@geoand we have merged what needed to be merged so you can have a look now. First let's see if it's not a whole new patch that needs to be written because in this case, we might not backport.

@geoand
Copy link
Contributor Author

geoand commented May 12, 2022

I'll take a look and let you know

@geoand
Copy link
Contributor Author

geoand commented May 16, 2022

I took a look at this and to be honest, I'd rather postpone cherry picking until we really need this.

@rsvoboda
Copy link
Member

There is JIRA for the issue considered for 2.7.z, so the backport would be welcome.

What are the troubles with the backport? Many changes happened to the ClientSendRequestHandler.java file?

@geoand
Copy link
Contributor Author

geoand commented May 16, 2022

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reactive Rest client can not upload files bigger, than 2G
4 participants