[uss_qualifier] scd CRSimple checks that mutation is only allowed with the correct OVN #765
Conversation
Shastick
force-pushed
the
cr-simple-mutation-scenario
branch
4 times, most recently
from
50fc09e
to
703ac55
Compare
| if qe.cause_status_code in [400, 404, 409]: | ||
| # An empty OVN can be seen as: | ||
| # an incorrect parameter (400), a reference to a non-existing entity (404) as well as a conflict (409) |
There was a problem hiding this comment.
Looking at the OpenAPI definition, I don't think it is correct to add a 404 here. The description is The requested Entity could not be found. The OVN is not an entity.
What motivates this addition?
There was a problem hiding this comment.
This comment on the equivalent PR for OIRs, so I'm aligning this scenario with it too.
We don't request from an implementation that they send a 404, but returning a 404 would not be entirely wrong either, and we generally want to have a reasonably wide interpretation.
There was a problem hiding this comment.
I agree there is some ambiguity in the 404 language. It could be read as "The requested Entity could not be found", meaning that the Entity existing and just not having the specified OVN should not return 404, but rather necessarily 400 or 409. The way I read it is "The requested Entity could not be found", meaning that the Entity, as requested (where request includes specification of the OVN -- essentially a subindex), could not be found. So, I would keep the 404 addition as that also matches the conceptual REST paradigm of a subfolder referring to subresources, and a 404 is the appropriate RESTful response to the absence of the requested subresource. I unfortunately can't make the meeting this week, but I don't feel super strongly about this if there is strong disagreement.
| ) | ||
| except QueryError as qe: | ||
| self.record_queries(qe.queries) | ||
| if qe.cause_status_code in [400, 404, 409]: |
There was a problem hiding this comment.
Same reply as above
Shastick
force-pushed
the
cr-simple-mutation-scenario
branch
from
703ac55
to
1795bbf
Compare
|
Note: prober step failed while pulling the base image: I start observing these once or twice a week. |
Shastick
force-pushed
the
cr-simple-mutation-scenario
branch
from
1795bbf
to
cc92286
Compare
…h the correct OVN
Shastick
force-pushed
the
cr-simple-mutation-scenario
branch
from
cc92286
to
fac668a
Compare
last piece to implement #760 by having a test case explicitly dedicated to attempting mutation with missing an incorrect OVNs
Only the last commit of this PR is relevant, this will remain a draft until #762 is merged.