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

No easy mechanism for getting/providing an ETag #86

Closed
imjasonh opened this issue Jun 25, 2015 · 16 comments
Closed

No easy mechanism for getting/providing an ETag #86

imjasonh opened this issue Jun 25, 2015 · 16 comments
Assignees
Labels
🚨 This issue needs some love. triage me I really want to be triaged.

Comments

@imjasonh
Copy link
Contributor

Here's my use case: I start a compute instance with svc.Instances.Insert(...).Do() which returns an Operation resource. I want to periodically poll on this operation to determine whether it's updated, but I don't really care to receive the whole resource each time I poll, and I know that Google APIs support the If-None-Match header to trigger an HTTP 304 Not Modified response and avoid sending the resource again.

This is just an example; I may be polling the contents of a folder in Drive, or events in a Calendar, or really anything that can change its state. If I don't care about the resource and just want to know whether it's changed, I should use an ETag.

In generated clients, there's no way to get the ETag from a previous request, and no way to provide one in the headers of subsequent requests. The APIs Explorer shows that at least the ETag response header is sent in the response, so it's available, just not exposed in any way via the generated clients.

More info:

@gmlewis gmlewis self-assigned this Jun 25, 2015
@gmlewis
Copy link
Contributor

gmlewis commented Jun 25, 2015

Interesting note... while investigating this issue, I discovered that several APIs already expose the Etag information to the caller. For example, this already exists in storage:v1, drive:v2, and calendar:v3.

Now investigating how to support the If-None-Match and If-Match.

@gmlewis
Copy link
Contributor

gmlewis commented Jun 25, 2015

@imjasonh please take a look at https://code-review.googlesource.com/2970
and let me know if you think this will provide the support that you are looking for.

@imjasonh
Copy link
Contributor Author

Thanks for the quick reply! The change looks like it covers the conditional
response case, but unless I've missed something it doesn't seem to give me
a way to determine the etag after making a request.

IsError is a nice addition too! :)
On Thu, Jun 25, 2015 at 5:25 PM Glenn Lewis notifications@github.com
wrote:

@imjasonh https://github.com/ImJasonH please take a look at
https://code-review.googlesource.com/2970
and let me know if you think this will provide the support that you are
looking for.


Reply to this email directly or view it on GitHub
#86 (comment)
.

@gmlewis
Copy link
Contributor

gmlewis commented Jun 25, 2015

Thanks, @imjasonh . Check out my comment above... I believe all the APIs of interest already return the Etag values. Do a grep on "Etag" over all the APIs, and you will see a lot.

@imjasonh
Copy link
Contributor Author

Ah, I see what you're saying. But then it's on each API provider to pass
the etag in the response body, where it can make it through to the decoded
structs, when all APIs already return an etag in the response header.
On Thu, Jun 25, 2015 at 5:52 PM Glenn Lewis notifications@github.com
wrote:

Thanks, @imjasonh https://github.com/ImJasonH . Check out my comment
above... I believe all the APIs of interest already return the Etag values.
Do a grep on "Etag" over all the APIs, and you will see a lot.


Reply to this email directly or view it on GitHub
#86 (comment)
.

@gmlewis
Copy link
Contributor

gmlewis commented Jun 25, 2015

I actually did perform some testing on the Calendar and Storage APIs, and could not find any Etags passed in the header, but then stumbled across the Etags within the API responses themselves.

@imjasonh
Copy link
Contributor Author

I see an ETag response header in a request to compute.instances.list in the APIs Explorer, for example. Being able to conditionally query this endpoint would let me poll for changes to the list without having to receive the actual response.

screen shot 2015-06-25 at 6 06 22 pm

@gmlewis
Copy link
Contributor

gmlewis commented Jun 25, 2015

Ah! So compute:v1 is one of the APIs that doesn't provide an explicit Etag in the response struct but instead leaves it in the header. OK, I'll see what I can do. Thanks for the example.

@gmlewis
Copy link
Contributor

gmlewis commented Jun 25, 2015

Wow, this could potentially really mess up the Do() API! What are the chances that we could get the Compute team to add an Etag string to their compute.targetInstances.list response?

@imjasonh
Copy link
Contributor Author

Yeah, I agree it's kind of a big change. So far the client hasn't exposed
anything about the response but the body.

It's not just that API, or even just Compute; every API would need to add
the etag to their responses, it's a pretty big non-starter.

Is there any other way to introspect into the raw HTTP response? Maybe I
can hack something up by injecting my own header-grabbing Transport? :(
On Thu, Jun 25, 2015 at 6:17 PM Glenn Lewis notifications@github.com
wrote:

Wow, this could potentially really mess up the Do() API! What are the
chances that we could get the Compute team to add an Etag string to their
compute.targetInstances.list response?


Reply to this email directly or view it on GitHub
#86 (comment)
.

@gmlewis
Copy link
Contributor

gmlewis commented Jun 25, 2015

I suppose we could add a DoWithHeader() that returns 3 values... the response, the header, and an error. I would have to get @bradfitz approval for that. (Do() would call DoWithHeader() and just return the response and error like it currently does.) Brad, thoughts?

@gmlewis
Copy link
Contributor

gmlewis commented Jun 26, 2015

@imjasonh - I've gone ahead and implemented DoHeader, hoping that Brad will like it.
Please take a look at https://code-review.googlesource.com/#/c/2970/3
I've also updated the compute.go example and tested it out by grabbing the Etag value from the header and then using it in a call to IfNoneMatch, and it all seems to work.

@imjasonh
Copy link
Contributor Author

That change looks good to me, thanks for putting it together. Two small questions:

  1. I would have expected the header to be returned second instead of first, but I don't feel strongly about it if you think that's more idiomatic.
  2. Is there any reason to make it DoResponse and return the whole http.Response instead of just headers? This would give a caller access to the underlying Request that was made (with empty body) and maybe some other useful stuff.

But this looks fine to me if it passes Brad's muster. Thanks again!

@gmlewis
Copy link
Contributor

gmlewis commented Jun 26, 2015

Re#1 - good point... I will switch it.

Re#2 - returning the full response seems much riskier to me because of the closed Body.

Let me fix the first point then I will send it out to Brad for review and let's see what he says about the second point.

@imjasonh
Copy link
Contributor Author

imjasonh commented Oct 2, 2015

Thanks Glenn! 👍

@gmlewis
Copy link
Contributor

gmlewis commented Oct 2, 2015

You're welcome, Jason!

@yoshi-automation yoshi-automation added 🚨 This issue needs some love. triage me I really want to be triaged. labels Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚨 This issue needs some love. triage me I really want to be triaged.
Projects
None yet
Development

No branches or pull requests

3 participants