ietf-wg-scitt / draft-ietf-scitt-scrapi

Transparency Service REST API
https://datatracker.ietf.org/doc/draft-ietf-scitt-scrapi/
Other
2 stars 6 forks source link

Add 'chck operation' endpoint #35

Closed JAG-UK closed 2 weeks ago

JAG-UK commented 2 months ago

Fixes #23

SteveLasker commented 2 months ago

Editors meeting suggestion: add a correlation id for the registration to coordinate on long-running operation requests.

SteveLasker commented 2 months ago

Question from the editor's call: Was the request ID also meant to be the operationId, or was it intended to be a tuple where the client can submit a unique ID that needs to be validated? However, can the server return a guaranteed unique ID? The client submitting a unique ID is part of the idempotent design pattern.

Some background on idempotent patterns

Do we need a request ID (Idempotency-Key)?

  1. If the client makes a registration request and fails to get a response, the client is unsure if the registration was received, the response failed, or the request was never received.
  2. The client would try again, which could generate multiple registrations

In an idempotent world, the server would check if the request was previously made and, if so, return the same response. The issue is that a client could be intentionally making a secondary registration because the registration policy may have changed or the client needs a new registration for a new point in time.

Separately, a discussion to support success/failure response codes for failed registration vs. "not found" is needed.

JAG-UK commented 2 months ago

Question from the editor's call: Was the request ID also meant to be the operationId, or was it intended to be a tuple where the client can submit a unique ID that needs to be validated? However, can the server return a guaranteed unique ID? The client submitting a unique ID is part of the idempotent design pattern.

The addition of the request_id in Commit 4 came out of the discussion in the prior editors' call where a couple of people asked for some kind of a correlation ID that the client could use to keep its own track of the LROs it was checking and receiving. Various use cases were mooted all along the lines that contexts may be confused (in the case of connection pooling or similar at the client end) or lost (in the case of one end or other crashing).

This seemed reasonable to try to accommodate and I did it to show how it might look, but actually I don't like how it's turned out and would gladly revert. Reasons:

In an idempotent world, the server would check if the request was previously made and, if so, return the same response. The issue is that a client could be intentionally making a secondary registration because the registration policy may have changed or the client needs a new registration for a new point in time.

I like the idempotency key stuff and we should look into it but I think it's a reasonable and coherent v1 to just say that EVERY POST is a new registration and starts a new registration operation for a new receipt. We can't legislate for software bugs or try to guess the intention of the client.

Separately, a discussion to support success/failure response codes for failed registration vs. "not found" is needed.

Agreed. That's already on the list.

ad-l commented 2 months ago

I like the idempotency key stuff and we should look into it but I think it's a reasonable and coherent v1 to just say that EVERY POST is a new registration and starts a new registration operation for a new receipt. We can't legislate for software bugs or try to guess the intention of the client.

Yes, I think I am convinced by this argument, the request_id returned for the /operations call is a unique identifier for the registration and is enough for clients. The ability to set request_id in the registration request is unnecessary and may be confusing. I would also suggest to rename request_id to registration_id to make it clear that it is the identifier picked by the server when the registration is accepted

ad-l commented 2 months ago

@JAG-UK here are my proposed changed to resolve this

https://github.com/ietf-wg-scitt/draft-ietf-scitt-scrapi/compare/jag-uk/23-distinct-async-operations...ad-l:draft-ietf-scitt-scrapi:jag-uk/23-distinct-async-operations

SteveLasker commented 2 weeks ago

Meeting notes: This PR dealt with several aspects, making it a bit harder to merge. Jon will create separate PRs, based on work for 121 hackathon.