feat (AIP-136) Update guidance of billable stateless methods #1124

xdtdaniel · 2023-05-31T19:29:35Z

Proposes changes for AIP-136 regarding the use of GET and POST for stateless methods.

toumorokoshi

Thanks! the guidance LGTM.

I'll add another API design reviewer explicitly.

toumorokoshi · 2023-06-02T20:04:41Z

aip/general/0136.md

+### Choice of HTTP methods for stateless methods
+
+Stateless method **should** use `GET` HTTP method for retrieval use cases due to
+its cache-friendly nature. This can reduce the costs for both the server and the


it might be worth elaborating: e.g. it's common for web browsers to cache GET requests, and it's understood by clients to be a cacheable response.

Thanks for the review! Can you elaborate a bit more on "and it's understood by clients to be a cacheable response" part? What does this mean? Specifically what is "it" referring to? Thanks!

Sorry, by "it" I was referring to the fact that GET methods are cacheable.

and it's understood by clients to be a cacheable response

What I mean is HTTP clients often cache GET responses as GET implies that it's doing data retrieval: https://datatracker.ietf.org/doc/html/rfc7231#section-4.3.1. Specficially:

The response to a GET request is cacheable; a cache MAY use it to
satisfy subsequent GET and HEAD requests unless otherwise indicated
by the Cache-Control header field (Section 5.2 of [RFC7234]).

So custom methods which are cacheable and send no payload should use GET.

Thanks, I edited it again with a new commit. PTAL!

toumorokoshi · 2023-06-02T20:05:35Z

@alin04 could you take a look?

aip/general/0136.md

toumorokoshi · 2023-06-09T16:09:16Z

@xdtdaniel make sure to re-request review once changes are addresses so it'll notify the reviewer.

@alin04 I think a new PR is up for your review.

xdtdaniel · 2023-06-09T16:33:37Z

Thanks! Sorry wasn't very familiar with this. Thanks for the notice! Yusuke Tsutsumi ***@***.***> 于 2023年6月9日周五 09:09写道：

…

@xdtdaniel <https://github.com/xdtdaniel> make sure to re-request review once changes are addresses so it'll notify the reviewer. @alin04 <https://github.com/alin04> I think a new PR is up for your review. — Reply to this email directly, view it on GitHub <#1124 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AI7GLMXZXHXKGY4WEO3AS73XKNDDRANCNFSM6AAAAAAYV5KURE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

aip/general/0136.md

toumorokoshi

Thanks! LGTM.

noahdietz · 2023-07-12T19:33:41Z

Sorry to nit pick when I wasn't asked for a review, but @toumorokoshi should we try to separate the guidance and the Rationale from each other? There is a lot of reasoning in the guidance, which distracts from the piece of guidance itself. Add me as a reviewer if you want this specific feedback.

loudej · 2023-07-13T08:51:14Z

aip/general/0136.md

+HTTP clients such as browsers often cache `GET` responses as `GET` implies that
+it is doing data retrieval. Stateless methods that are cacheable and send no
+payload **should** use `GET` HTTP method due to its cache-friendly nature. 
+This can reduce the costs for both the server and the client.


Unfortunately, I don't believe cacheability to informs the choice of method for rest api.

On one hand, per RFC the response returned by non-GET operations may carry an entity that has cacheable implications.

https://datatracker.ietf.org/doc/html/rfc7231#section-7.2

In a successful response to a state-changing request, validator fields describe the new representation that has replaced the prior selected representation as a result of processing the request. For example, an ETag header field in a 201 (Created) response communicates the entity-tag of the newly created resource's representation, so that it can be used in later conditional requests to prevent the "lost update" problem [[RFC7232](https://datatracker.ietf.org/doc/html/rfc7232)].

So the litmus test is less like "is your response cacheable?" and more like "is the URI (path+query) the address of a data or of a function?" --- and if the URI is the address of data --- do all of the idempotent and non-idempotent methods acting on that URI comply with the RFC necessary to be cacheable?

And that's the other hand, our API are actually miles away from cacheable and would take effort to change that... Like:

return an etag json field has no semantic meaning - the response would need to contain an ETag response header instead

the format of our ETag isn't right. it doesn't matter if it's a guid or not - but the string value must contain starting and ending with double-quote characters

both the mutating and non-mutating URI would need to have the same path+query for cache hits, cache invalidation, and optimistic concurrency. the way we have processing-instruction parameters appear as query arguments (like update_mask=xxx and etag=yyy) means that PATCH and DELETE operations are correlated to different cache entries than the GET's response represents

and speaking of ?etag=yyy precondition field --- for browser and httpclient cacheability what we'd need to accept are the If-Match yyy and If-None-Match: yyy request header parameters.

(I think those would be great to do --- but without that using a cache-enabled api httpclient does more harm than good. w/out etag and 304 response code it can actively prevent you from reaching past stale data --- especially if PUT/PATCH/DELETE aren't invalidating cache entries by URI properly)

(Also worth noting this isn't unusual --- many (most?) (almost all??) systems with REST API interfaces don't go down the road of http-defined cachability. I suppose that's because (unlike a 70k js or png file) by the time you've paid for the round-trip, done the mandatory authn/authz for the operation, and drawn sufficient info from the DB to known that the etag verifier is up-to-date ---- returning 304 and saving the typically smaller response of an average API doesn't have much ROI left)

Thanks Louis for the comment. I think it makes sense to emphasize the part of how uri informs the operation - is it more of a data retrieval or something more complicated than that. Let me rephrase this part. I will wait for Noah's comment as well to make the change together. Thanks!

xdtdaniel · 2023-07-14T20:06:15Z

Sorry to nit pick when I wasn't asked for a review, but @toumorokoshi should we try to separate the guidance and the Rationale from each other? There is a lot of reasoning in the guidance, which distracts from the piece of guidance itself. Add me as a reviewer if you want this specific feedback.

Hi @noahdietz can you help adding yourself as a reviewer? I am not sure if I can add you as a reviewer. Thanks!

noahdietz

I realize that this API doesn't have a Rationale section (see AIP-8) yet, but we should add one with this change to separate out the guidance from the reason why this guidance is necessary/beneficial.

noahdietz · 2023-07-14T20:06:34Z

aip/general/0136.md

+HTTP clients such as browsers often cache `GET` responses as `GET` implies that
+it is doing data retrieval. Stateless methods that are cacheable and send no


HTTP clients such as browsers often cache `GET` responses as `GET` implies that it is doing data retrieval.

This is rationale, not guidance

noahdietz · 2023-07-14T20:06:58Z

aip/general/0136.md

+
+HTTP clients such as browsers often cache `GET` responses as `GET` implies that
+it is doing data retrieval. Stateless methods that are cacheable and send no
+payload **should** use `GET` HTTP method due to its cache-friendly nature. 


due to its cache-friendly nature.

This is rationale, not guidance.

noahdietz · 2023-07-14T20:07:19Z

aip/general/0136.md

+HTTP clients such as browsers often cache `GET` responses as `GET` implies that
+it is doing data retrieval. Stateless methods that are cacheable and send no
+payload **should** use `GET` HTTP method due to its cache-friendly nature. 
+This can reduce the costs for both the server and the client.


This can reduce the costs for both the server and the client.

This is rationale, not guidance.

Update 0136.md for guidance of billable stateless methods

c9105e7

Proposes changes for AIP-136 regarding the use of GET and POST for stateless methods.

xdtdaniel requested a review from a team as a code owner May 31, 2023 19:29

toumorokoshi approved these changes Jun 2, 2023

View reviewed changes

toumorokoshi requested a review from alin04 June 2, 2023 20:05

Merge branch 'master' into master

4871799

Clarify on the why the cacheable requests should use GET

7fd7b34

alin04 reviewed Jun 6, 2023

View reviewed changes

aip/general/0136.md Outdated Show resolved Hide resolved

aip/general/0136.md Show resolved Hide resolved

xdtdaniel added 2 commits June 6, 2023 20:41

Update 0136.md

c43b4c5

Update 0136.md to reflect current date

85f1829

toumorokoshi requested a review from alin04 June 9, 2023 16:08

Merge branch 'master' into master

4426da2

xdtdaniel requested a review from toumorokoshi June 10, 2023 05:08

Merge branch 'master' into master

0bcef51

toumorokoshi suggested changes Jul 5, 2023

View reviewed changes

aip/general/0136.md Outdated Show resolved Hide resolved

Update 0136.md

b2dcccc

xdtdaniel requested a review from toumorokoshi July 5, 2023 20:36

toumorokoshi approved these changes Jul 5, 2023

View reviewed changes

loudej reviewed Jul 13, 2023

View reviewed changes

noahdietz reviewed Jul 14, 2023

View reviewed changes

Merge branch 'master' into master

dd59db2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat (AIP-136) Update guidance of billable stateless methods #1124

feat (AIP-136) Update guidance of billable stateless methods #1124

xdtdaniel commented May 31, 2023

toumorokoshi left a comment

toumorokoshi Jun 2, 2023

xdtdaniel Jun 2, 2023

toumorokoshi Jun 2, 2023

xdtdaniel Jun 3, 2023

toumorokoshi commented Jun 2, 2023

toumorokoshi commented Jun 9, 2023

xdtdaniel commented Jun 9, 2023 via email

toumorokoshi left a comment

noahdietz commented Jul 12, 2023

loudej Jul 13, 2023 •

edited

Loading

xdtdaniel Jul 14, 2023

xdtdaniel commented Jul 14, 2023

noahdietz left a comment

noahdietz Jul 14, 2023

noahdietz Jul 14, 2023 •

edited

Loading

noahdietz Jul 14, 2023

		HTTP clients such as browsers often cache `GET` responses as `GET` implies that
		it is doing data retrieval. Stateless methods that are cacheable and send no

feat (AIP-136) Update guidance of billable stateless methods #1124

Are you sure you want to change the base?

feat (AIP-136) Update guidance of billable stateless methods #1124

Conversation

xdtdaniel commented May 31, 2023

toumorokoshi left a comment

Choose a reason for hiding this comment

toumorokoshi Jun 2, 2023

Choose a reason for hiding this comment

xdtdaniel Jun 2, 2023

Choose a reason for hiding this comment

toumorokoshi Jun 2, 2023

Choose a reason for hiding this comment

xdtdaniel Jun 3, 2023

Choose a reason for hiding this comment

toumorokoshi commented Jun 2, 2023

toumorokoshi commented Jun 9, 2023

xdtdaniel commented Jun 9, 2023 via email

toumorokoshi left a comment

Choose a reason for hiding this comment

noahdietz commented Jul 12, 2023

loudej Jul 13, 2023 • edited Loading

Choose a reason for hiding this comment

xdtdaniel Jul 14, 2023

Choose a reason for hiding this comment

xdtdaniel commented Jul 14, 2023

noahdietz left a comment

Choose a reason for hiding this comment

noahdietz Jul 14, 2023

Choose a reason for hiding this comment

noahdietz Jul 14, 2023 • edited Loading

Choose a reason for hiding this comment

noahdietz Jul 14, 2023

Choose a reason for hiding this comment

loudej Jul 13, 2023 •

edited

Loading

noahdietz Jul 14, 2023 •

edited

Loading