[batch] 400 on bad job group specs instead of 500 #14389
Conversation
daniel-goldstein
force-pushed
the
400-on-invalid-spec
branch
from
e03eca5 to
a689656
Compare
daniel-goldstein
force-pushed
the
400-on-invalid-spec
branch
from
a689656 to
da3ef9e
Compare
batch/batch/front_end/front_end.py
Outdated
| try: | ||
| validate_job_groups(job_group_specs) | ||
| except ValidationError as e: | ||
| raise web.HTTPBadRequest(reason=e.reason) |
There was a problem hiding this comment.
Could this be moved up the stack? I feel like once execution reaches this point, the request itself should have been well-formed at least?
There was a problem hiding this comment.
Ya it could be moved up one level to the route handler. I changed the other resource creation methods to match. Slightly repetitive, but ensures we never touch the database if anything about the request is malformed. Unfortunately we're not using strict mode for pyright and json_request returns Any (so passing its output to other methods will always pass the typechecker regardless of the signature) so I'm not so sure how we can enforce in the type checker that the specs were validated.
There was a problem hiding this comment.
Do we use an openapi spec for the batch http api? I've used tools that parse requests against a spec and only pass valid requests onto you in other environments - doubtless something like this exists for python.
There was a problem hiding this comment.
we certainly could, but we no we don't it's pretty vanilla
Fixes #14350