Skip to content

fix: 🐛 replace accept-encoding application/json to null for all clients#205

Merged
bednar merged 1 commit intoinfluxdata:masterfrom
antoinedvd:fix/remove-accept-encoding
Mar 15, 2021
Merged

fix: 🐛 replace accept-encoding application/json to null for all clients#205
bednar merged 1 commit intoinfluxdata:masterfrom
antoinedvd:fix/remove-accept-encoding

Conversation

@antoinedvd
Copy link
Copy Markdown
Contributor

@antoinedvd antoinedvd commented Mar 12, 2021 •
edited
Loading

Proposed Changes

👋 Hi folks, I try make a humble contribution to fix a gzip compression issue during a query.

  • the Accept-Encoding header is not used for influx 2.0 server because it always send back CSV data (the server didn't care if you asked for JSON)
  • this header blocks the Accept-Encoding: gzip header from the BridgeInterceptor (quick look here 👀)

Checklist

  • CHANGELOG.md updated
  • Rebased/mergeable
  • A test has been added if appropriate
  • mvn test completes successfully
  • Commit messages are in semantic format
  • Sign CLA (if not already signed)

@antoinedvd antoinedvd changed the title fix(kotlin-client): 🐛 replace accept-encoding application/json to… fix(kotlin-client): 🐛 replace accept-encoding application/json to null Mar 12, 2021
@antoinedvd antoinedvd force-pushed the fix/remove-accept-encoding branch from ceb775c to 5115baf Compare March 12, 2021 13:10
@codecov-io
Copy link
Copy Markdown

codecov-io commented Mar 12, 2021 •
edited
Loading

Codecov Report

Merging #205 (5115baf) into master (c6aaa57) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #205   +/-   ##
=========================================
  Coverage     88.44%   88.44%           
  Complexity      447      447           
=========================================
  Files           139      139           
  Lines          5512     5512           
  Branches        226      226           
=========================================
  Hits           4875     4875           
  Misses          560      560           
  Partials         77       77
Impacted Files Coverage Δ Complexity Δ
...luxdb/client/kotlin/internal/QueryKotlinApiImpl.kt 94.02% <100.00%> (ø) 0.00 <0.00> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c6aaa57...5115baf. Read the comment docs.

@antoinedvd antoinedvd marked this pull request as ready for review March 12, 2021 13:16
bednar
bednar requested changes Mar 12, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@bednar bednar left a comment

Choose a reason for hiding this comment

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

Thanks again for your PR 👍

Please, could you also update following APIs:

?

@antoinedvd
Copy link
Copy Markdown
Contributor Author

antoinedvd commented Mar 12, 2021

Thanks again for your PR +1

Please, could you also update following APIs:

* https://github.com/antoinedvd/influxdb-client-java/blob/5115baff7dd863b4119f5c40514ddba2db6e2e69/client/src/main/java/com/influxdb/client/internal/QueryApiImpl.java#L796

* https://github.com/antoinedvd/influxdb-client-java/blob/5115baff7dd863b4119f5c40514ddba2db6e2e69/client/src/main/java/com/influxdb/client/internal/QueryApiImpl.java#L814

* https://github.com/antoinedvd/influxdb-client-java/blob/5115baff7dd863b4119f5c40514ddba2db6e2e69/client-reactive/src/main/java/com/influxdb/client/reactive/internal/QueryReactiveApiImpl.java#L121

* https://github.com/antoinedvd/influxdb-client-java/blob/5115baff7dd863b4119f5c40514ddba2db6e2e69/client-reactive/src/main/java/com/influxdb/client/reactive/internal/QueryReactiveApiImpl.java#L259

* https://github.com/antoinedvd/influxdb-client-java/blob/5115baff7dd863b4119f5c40514ddba2db6e2e69/client-scala/src/main/scala/com/influxdb/client/scala/internal/QueryScalaApiImpl.scala#L105

* https://github.com/antoinedvd/influxdb-client-java/blob/5115baff7dd863b4119f5c40514ddba2db6e2e69/client-scala/src/main/scala/com/influxdb/client/scala/internal/QueryScalaApiImpl.scala#L280

?

Yes, I'll do that. 😉

fix: 🐛 replace accept-encoding application/json to null on all clients
efa46cf 
- this header is not used for influx 2.0 server because it always send back CSV data
- this header blocks the accept-encoding gzip header from the BridgeInterceptor
@antoinedvd antoinedvd force-pushed the fix/remove-accept-encoding branch from 5115baf to efa46cf Compare March 12, 2021 13:36
@antoinedvd antoinedvd changed the title fix(kotlin-client): 🐛 replace accept-encoding application/json to null fix: 🐛 replace accept-encoding application/json to null for all clients Mar 12, 2021
@antoinedvd
Copy link
Copy Markdown
Contributor Author

antoinedvd commented Mar 12, 2021

@bednar it's done ✅

@antoinedvd antoinedvd mentioned this pull request Mar 12, 2021
Closed
bednar
bednar approved these changes Mar 15, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@bednar bednar left a comment

Choose a reason for hiding this comment

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

Thanks again for your PR 👍

@bednar bednar merged commit d2936fb into influxdata:master Mar 15, 2021
@bednar bednar added this to the 2.1.0 milestone Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bednar bednar bednar approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2.1.0

Development

Successfully merging this pull request may close these issues.

3 participants

@antoinedvd @codecov-io @bednar