aip-dev / google.aip.dev

API Improvement Proposals. https://aip.dev/
Other
1.1k stars 501 forks source link

AIP-155: consider removing #1103

Open toumorokoshi opened 1 year ago

toumorokoshi commented 1 year ago

If #1019 is merged, the resource ID could act as the idempotency mechanism for a resource. See https://github.com/aip-dev/google.aip.dev/pull/1019#discussion_r1192860620 for more information.

noahdietz commented 1 year ago

There are several other situations outside of Standard Create that might want to expose a request_id to enable idempotency. The linked discussion only focuses on Standard Creates with a {resource}_id field to effectively use as the request_id.

What about Delete? What about Update? What about Custom methods?

noahdietz commented 1 year ago

Moving to backlog as I don't think this should be in scope of CS-2312 for now. AFAICT it doesn't inhibit surfaces in anyway.

bgrant0607 commented 1 year ago

Update, delete, and side-effecting custom methods should use etag.

Only operations that don't mutate resource attributes need to use request_id -- data plane.

noahdietz commented 1 year ago

Update, delete, and side-effecting custom methods should use etag.

Based on their respective AIPs, I don't think etag and request_id are meant for the same use case. etag is for guaranteeing freshness and request_id is for enabling server-side deduping of retried/hedged requests. They can even be used together. I think that they elicit different enough responses that the user flows are not interchangeable.

Take an example like UpdateBook to change the title field. In a lost-connection retry scenario, the client receives an Unavailable error, but the service had already applied the update.

I think request_id/AIP-155 has a place in Standard Methods and is widely used today for that reason.

toumorokoshi commented 1 year ago

If etag were used, a retried request would produce an ABORTED (per AIP-154), because the backend updated the committed etag after processing the (broken) first call. If request_id were used, a retried request would produce an OK (per AIP-155), because the backend already processed that same request and it was successful.

I think the question on this is not that these are indeed different, but if the difference introduces any new user journeys.

I'd treat both the same: if a request fails, I retry it, and if it doesn't succeed, I try again. Is there a situation where the remedial action I would take as a user would be different with an etag vs request_id?

noahdietz commented 1 year ago

I'm not sure I follow. The point is I was making is that when presented with the same scenario, one field would produce a different user journey (etag: sees aborted, needs to modify request before retrying) than the other (request_id, gets OK because the server was actually able to process it already and it replayed the response), and both are valid user journeys.

bgrant0607 commented 1 year ago

I would never recommend request_id for any method that resulted in a change to resource attributes that would be reflected in etags.

Creation currently has a gap because if the resource came into existence, it isn't clear whether it matches the requester's intent, but a declarative tool would then diff the state and update if necessary anyway. That's better solved by Apply.