Skip to content
This repository has been archived by the owner on Jul 24, 2021. It is now read-only.

API wrappers for Create endpoints (POSTS that return a redirect) should return the redirected data #89

Open
perigrin opened this issue Dec 2, 2020 · 4 comments
Assignees
Labels
Breaking enhancement New feature or request WAT causes user confusion
Milestone

Comments

@perigrin
Copy link
Contributor

perigrin commented Dec 2, 2020

Currently we ignore a redirect on create because it was simpler to implement. We should instead follow the redirect and do the lookup then return the data.

This will be a breaking change for the conch library code so needs to happen in 4.x.

@perigrin perigrin added enhancement New feature or request WAT causes user confusion Breaking labels Dec 2, 2020
@perigrin perigrin added this to the 4.0.0 milestone Dec 2, 2020
@perigrin perigrin self-assigned this Dec 2, 2020
@karenetheridge
Copy link
Contributor

is this really needed? If the shell doesn't need to see the new/changed data it doesn't need to follow the redirect, and that will save the api a needless query.

Last year I wanted to change all the 303 + URI responses to 201 and Location header, but sungo complained because the shell at that time expected to follow all redirects. Now that that constraint has been removed, do we need to bring it back?

@perigrin
Copy link
Contributor Author

perigrin commented Dec 3, 2020

I’m happy to have 201 + Location headers. For kosh this is only necessary because the racks can be created without knowing their UUID but I seem to only be able to look them up with their UUID which may be a bug to report to you after I dig some more.

That said I almost invariably do a second GET almost immediately after issuing a create from the cli so I can provide feedback to the user that their action did something. Probably either have that step be optional (e.g. new wrapper methods that do the fetch) or return the URI (or just the relevant bits) instead of doing the fetch automatically.

@karenetheridge
Copy link
Contributor

I believe all Location headers (used for both 201 and 303 responses) contain the canonical identifier for the resource, which would contain its UUID (there was one outlier that i fixed a few months ago where POST /hardware_vendor would return the name, not the uuid). However, unless kosh automatically follows 303 redirects, the response is the same either way - it's a Location header in both cases.

@perigrin
Copy link
Contributor Author

perigrin commented Dec 3, 2020

Either way I’m currently throwing away the results. I think I’d prefer a 201 and then I can make it so with the Go API you can choose if you want to follow or not.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Breaking enhancement New feature or request WAT causes user confusion
Projects
None yet
Development

No branches or pull requests

2 participants