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.

Sign up for GitHub

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

Jump to bottom

Allow more fine-grained control of compression setting for REST Client #46417

Merged
merged 1 commit into from
Feb 25, 2025
Merged

Allow more fine-grained control of compression setting for REST Client #46417

merged 1 commit into from
Feb 25, 2025

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Feb 21, 2025 •
edited
Loading

@geoand geoand mentioned this pull request Feb 21, 2025
Closed
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 21, 2025 •
edited
Loading

🙈 The PR is closed and the preview is expired.

@geoand geoand marked this pull request as ready for review February 25, 2025 14:22
@geoand geoand requested a review from cescoffier February 25, 2025 14:23
@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Feb 25, 2025 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 9ba4ed8.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

cescoffier
cescoffier approved these changes Feb 25, 2025
View reviewed changes
Copy link
Member

@cescoffier cescoffier left a comment

Choose a reason for hiding this comment

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

LGTM

Any reason why the compression is handled using an interceptor and not the underlying client?

docs/src/main/asciidoc/rest-client.adoc
@@ -1755,9 +1755,22 @@ The code uses the following pieces:
As previously mentioned, the body parameter needs to be properly crafted by the application code to conform to the service's requirements.

=== Receiving compressed messages
REST Client also supports receiving compressed messages using GZIP. You can enable the HTTP compression support by adding the property `quarkus.http.enable-compression=true`.
REST Client also supports receiving compressed messages using GZIP and can be enabled via configuration.
Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised it does not handle the other compression protocols like Brotli.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also something to check as a follow up

.../client/runtime/src/main/java/org/jboss/resteasy/reactive/client/impl/ClientBuilderImpl.java
@@ -278,7 +278,7 @@ public ClientImpl build() {
}
}

if (enableCompression) {
if (Boolean.TRUE.equals(enableCompression)) {
Copy link
Member

Choose a reason for hiding this comment

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

Ah that's why. It is done using an interceptor.
Any reason for this?

@geoand
Copy link
Contributor Author

geoand commented Feb 25, 2025

Any reason why the compression is handled using an interceptor and not the underlying client?

That's a good question, I didn't do the initial implementation, so I'll have to check that as a follow-up

@geoand geoand merged commit 8688a6e into quarkusio:main Feb 25, 2025
57 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.21 - main milestone Feb 25, 2025
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Feb 25, 2025
@geoand geoand deleted the #46415 branch February 25, 2025 18:54
@geoand
Copy link
Contributor Author

geoand commented Feb 25, 2025

I'll be looking at #46496 this week

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cescoffier cescoffier cescoffier approved these changes

Assignees
No one assigned
Labels
area/documentation area/rest area/rest-client area/resteasy-classic kind/enhancement New feature or request
Projects
None yet
Milestone
3.21.0.CR1
Development

Successfully merging this pull request may close these issues.

Enable HTTP compression for REST client only
2 participants
@geoand @cescoffier