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

Link vs Location #57

Closed
StevenXL opened this issue Jan 8, 2021 · 10 comments · Fixed by #63
Closed

Link vs Location #57

StevenXL opened this issue Jan 8, 2021 · 10 comments · Fixed by #63

Comments

@StevenXL
Copy link
Contributor

StevenXL commented Jan 8, 2021

In Status Codes, should "202 for async jobs (with Link header)" read "202 for async jobs (with Location header)"?

@pbrisbin
Copy link
Member

pbrisbin commented Jan 8, 2021

I'm not sure what it should be, but I agree that it shouldn't be Link.

We don't have async Jobs, or use 202 yet. Microsoft Guides, which I propose we follow if/when we implement such functionality, talks about 202 and uses the non-standard Operation-Location header. But there is active discussion about using Content-Location (also a standard header) instead, and I'm in favor of that, personally.

@pbrisbin
Copy link
Member

@StevenXL I think we should decide if we want to follow the MS Guide as-is, or follow the MS Guide + that open Issue. And then I will fix the our Guide.

There are 3 kinds of long running operations responses being discussed: PUT, POST, and POST+hybrid. I'll refer to PUT and POST as Normal here, since that distinction doesn't matter in this discussion. Normal would be a request that leads to creation of something that you have to wait for, as you might guess. Hybrid means that some resource was created right away, but it's not ready. Imagine a request to create and then migrate a database. The DB is created, but you are waiting on the migration operation.

  1. Guide as-is: https://github.com/microsoft/api-guidelines/blob/vNext/Guidelines.md#132-stepwise-long-running-operations

    Flow Status Link to operation Link to resource
    Normal 202 Operation-Location N/A
    Hybrid 201 Operation-Location Location
  2. Guide + open Issue: https://github.com/microsoft/api-guidelines/blob/1dbb50971caf3ddfa68b1eeb25b270148b528d56/Guidelines.md#132-stepwise-long-running-operations

    Flow Status Link to operation Link to resource
    Normal 202 Location N/A
    Hybrid 202 Location Content-Location

My understanding of the benefits of (1) is:

  • Follows a strict interpretation of HTTP (e.g. something was created, you MUST use 201 + Location)

My understanding of the benefits of (2) is:

  • Uses standard HTTP headers (Content-Location is standard, Operation-Location is not)

The other factor is that the open Issue has been open for over 2 years. I fear it may never close, despite its popularity, due to the backwards-compatibility concern.

@eborden any thoughts?


We actually implemented our first API using this pattern, but we sort of missed on the response spec. What ended up happening was we return a 200 with body { operationId: X } in the response, rather than 202 with Header. This is not ideal, beyond just going off-spec, because now the Frontend has to know the route for some OperationId, instead being given an opaque route in a Header. I'd like to address this, but of course it relies on the conclusion of this issue. /cc @stackptr

@eborden
Copy link
Contributor

eborden commented Jul 20, 2021

Status

202 is the correct status to use here. The response has been accepted, but the product of that response is not yet processed.

Header

  • Link - This is a very generic header. An essential building block of HATEOAS. You can tag the context of each link you provide (Link: <https://api.freckle.com/2/operation/1>; rel="operation") allowing you to work within a domain specific grammar.
  • Location - The intent of this header is to communicate the location of the target resource. Primarily used for redirect, but can be used for 201 responses that don't return the resource directly. For example an API that generates a PDF may want to indicate a canonical CDN location for the created resource. Async operation progress isn't really the target resource, so this seems somewhat off the intent. Though the argument could be made that the operation is the target resource.
  • Content-Location - This is similar to Location, but for the content negotiation use case. This is really a caching hack to allow browser caches to short circuit content negotiation if the ideal response is already known and cache-able.

My Opinion

Status: 202
Link:  <https://api.freckle.com/2/operations/1>; rel="operation"

@pbrisbin
Copy link
Member

Awesome, thanks for the thoughts.

Let me repeat some of what you said back, to make sure I understand:

You're suggesting we don't follow either version of the MS Guides on this point. Option (1) misuses 201 and (2) misuses an (albeit Standard) HTTP header. And these are reason enough to go off-Guide.

You're not proposing anything for the Hybrid case, which I take to mean we leave it unspecified until/unless we have one.

The response has been accepted, but the product of that response is not yet processed

You're fully disagreeing with the way MS is interpreting the HTTP spec for the Hybrid flow, where (they say) the product of the response has been created (hence 201 + Location) but there is just more to do (hence an additional Operation-Location).

I have no real issues with using Link in a vacuum, but I hesitate to move away from the MS Guides. I think following a Guide that someone else maintains can really cut down on bikesheds. I truly look at this like forking a dependency, is it worth it?

I am a little wary on this point:

allowing you to work within a domain specific grammar

which could be restated as, "forcing you to define a domain specific grammar". But I like sharp tools, so I could get behind it. Hopefully the Link-handling code in our Frontend for pagination is sufficiently general to be used for this (Narrator voice: it wasn't.)

Though the argument could be made that the operation is the target resource.

If we took this view, we end up with:

Status: 202
Location: https://api.freckle.com/2/operations/1

Which, IMO, is simple, clear, and easy for a Frontend client to work with. It also happens to be (2), if you never have a Hybrid use-case. This was my original preference, because I didn't fully appreciate the Hybrid case and basically ignored it.


My opinion has changed a bunch while thinking about this, but I think I'm landing at using the MS Guide as-is, so (1). Reasons being,

  • 201 is totally defensible for a Hybrid flow scenario, IMO
  • Given that, Location is equally valid there
  • I wish they had done something other than the Operation-Location header, but it's just not a big enough downside to push me off-guide

The thought of this discussion ending with "do our own thing", effectively hand-cuffing us into future discussions, feels like a big downside. If instead this discussion ended with "follow that Guide", we're done here. And I believe that guide is plenty reasonable enough to follow, which is of course a prerequisite to that stance.

This is especially true for the Hybrid flow. We don't have any today, but when we do we'll need to re-open this topic all over again. Seems a little unnecessary when there's a ready-made pattern right there.

@eborden
Copy link
Contributor

eborden commented Jul 20, 2021

I'm super curious about why they differentiate in the hybrid use-case. Nothing in the HTTP spec says you can't return a body for a 202.

If instead this discussion ended with "follow that Guide", we're done here.

Let me decouple my own opinions and disagreements with what is pragmatic.

  • I think following the guide is the pragmatic choice. It has some things I see as odd, but I don't have a huge problem with them.
  • We also have a larger ecosystem of applications to think about in the Renaissance sphere and arguing for compliance with a community guide (especially from MS) is far easier to do than arguing for Freckle's opinions (my opinions?) on this matter.
  • We should just follow the guide.

@eborden
Copy link
Contributor

eborden commented Jul 20, 2021

Oh and by "we should just follow the guide" I mean in its current form. We shouldn't attempt to project the outcome of an active discussion.

@pbrisbin
Copy link
Member

We also have a larger ecosystem of applications to think about in the Renaissance sphere and arguing for compliance with a community guide (especially from MS) is far easier to do than arguing for Freckle's opinions (my opinions?) on this matter.

In case you missed it, I was introduced to this Guide through the RL Architects. RL APIs aspire to follow this exact guide. This is really why following this one in particular appeals to me. There are caveats though:

  • That push is recent and not particularly embedded yet
  • The vast majority of existing RL APIs likely do not align with the Guide
  • They're very far away from considering the finer points, such as this conversation we're having

@pbrisbin
Copy link
Member

If there is no objection from @StevenXL in the next day or so, I'm going to open a PR that updates that Status Code section to say something like:

202 for [Long-Running Operations]

And link it to the relevant section in the MS Guide, implying we should be following it as-is.

I plan to incorporate and link out to the MS Guide more from this Guide, as appropriate. See #61.

@eborden
Copy link
Contributor

eborden commented Jul 20, 2021

Sounds good!

@StevenXL
Copy link
Contributor Author

StevenXL commented Jul 20, 2021

After reading the comments, I agree with PR #61. The benefits of delegating any future discussion to MS Guide is a large benefit. PR good to go.

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

Successfully merging a pull request may close this issue.

3 participants