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

Fixes #1091 - Support gzip encoded requests #1092

Closed
wants to merge 1 commit into from
Closed

Fixes #1091 - Support gzip encoded requests #1092

wants to merge 1 commit into from

Conversation

orf
Copy link
Contributor

@orf orf commented Sep 14, 2018

I've never written any C++ nor used Bazel before, so please don't laugh if this is terrible!

I've had a go at supporting gzipped request bodies inside the tensorflow-serving http server: #1091

I've got no idea if the ticket will be accepted, but I thought it would be fun to have a go at fixing it.

I've also got no idea how to test this, I copied an existing test and made it used zlib (compatible with gzip) to send encoded data.

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@orf
Copy link
Contributor Author

orf commented Sep 14, 2018

I signed it!

@googlebot
Copy link

CLAs look good, thanks!

@netfs
Copy link
Collaborator

netfs commented Sep 20, 2018

Thanks for your effort and the PR!

@wenbozhu
Copy link
Contributor

Sorry for the delayed response. I am working on the tensorflow-serving http server and the proper fix will require a bit more work (to be spec compliant etc). We may not want to depend on the gzip wrapper library either.

Do you mind filing an issue (with a link to this PR) and I try to get the feature supported soon?

Thanks.

@orf
Copy link
Contributor Author

orf commented Sep 27, 2018

Sorry for the delayed response. I am working on the tensorflow-serving http server and the proper fix will require a bit more work (to be spec compliant etc). We may not want to depend on the gzip wrapper library either.

Do you mind filing an issue (with a link to this PR) and I try to get the feature supported soon?

Thanks.

I've filed a bug here: #1091

Yeah this will take more work, I guess this was just a POC more than anything else. I saw that Tensorflow itself has some gzip/zlib decoders, but they seem tightly coupled to files and hard to adapt here.

@gautamvasudevan
Copy link
Collaborator

With the issue filed, we'll track the work there. Thanks again!

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

Successfully merging this pull request may close these issues.

5 participants