Skip to content

adds: GZip compression feature (#591)#596

Merged
aseure merged 1 commit intomasterfrom
feat/gzip
Jul 10, 2019
Merged

adds: GZip compression feature (#591)#596
aseure merged 1 commit intomasterfrom
feat/gzip

Conversation

@Ant-hem
Copy link
Copy Markdown
Member

@Ant-hem Ant-hem commented Jul 5, 2019

Adds the possibility to compress request for SearchClient.
For the moment only GZIP compression is available.
GZIP compression is enabled by default for SearchClient.

Q A
Bug fix? no
New feature? yes
BC breaks? no
Related Issue Fix #591
Need Doc update yes

AccountClient test is expected to fail because the feature is not enabled on the second test server yet.

[changelog]
Adds the possibility to compress POST/PUT requests for SearchClient.
For the moment only GZIP compression is available.
GZIP compression is enabled by default for SearchClient.
Copy link
Copy Markdown

@aseure aseure left a comment

Choose a reason for hiding this comment

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

I'd say the PR is ready to merge as-is feature wise, hence my approval. Once thing you could consider though is to make the implementation not GZIP-specific. What I mean by that is that we chose to introduce an enum NONE/GZIP for the compression type instead of a boolean. However, when it comes to serialization, we don't switch among the different possibilities. This is definitely nitpicking as we don't plan to have any other compression format any time soon and it is clearly internal design, hence the approval :) Up to you buddy.

From a functional point of view, super good change! Thank you again for leading this with the engine team @Ant-hem 👍 👍 👍

@Ant-hem
Copy link
Copy Markdown
Member Author

Ant-hem commented Jul 10, 2019

Thanks for the comments @aseure! 🎉

What I mean by that is that we chose to introduce an enum NONE/GZIP for the compression type instead of a boolean.

I did the update with the enum for both Java and C# PRs. See for Java. Did I miss something else?

@aseure aseure merged commit e13ad77 into master Jul 10, 2019
@aseure aseure deleted the feat/gzip branch July 10, 2019 13:07
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.

API Feature - Implement GZIP for POST/PUT

2 participants