-
Notifications
You must be signed in to change notification settings - Fork 94
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
Scheduler command arg validation #6013
Comments
It is possible to validate a request and cause it to fail if needed, but only before the command is queued. Cylc used to have a layer to do this, it looked for a "proxy" command: cylc-flow/cylc/flow/network/resolvers.py Lines 733 to 735 in 5398941
If one was provided, it would call it rather than looking for a scheduler method: cylc-flow/cylc/flow/network/resolvers.py Lines 776 to 797 in 5398941
This was the vestigial remains of the old HTTP server where each command had a "proxy" in the server code: cylc-flow/lib/cylc/network/httpserver.py Lines 311 to 317 in 4e43fa5
The need for this layer dropped away thanks to the better type support and validation features of GraphQL so the "proxy" commands were removed on by one. The last remains of this interface were removed with #5658 as they were no longer used. To add validation back in, we'll need something like this layer. We could potentially re-implement this with a little syntactic sugar e.g: def validate(validate_fcn, command_fcn):
command_fcn.validate = validate_fcn
return command_fcn
def validate_set(args):
if ...:
raise InputError('some argument must be ....')
class Scheduler:
# ...
@validate(validate_set)
@def command set(...):
...
class Resolvers:
# ...
meth = getattr(schd, command)
if meth.validate:
try:
meth.validate(args)
except InputError as exc:
# TODO: return error
schd.command_queue.put((meth, args)) |
Yes that's the approach I'm taking. |
A detail:
|
Working on this today, PR tomorrow... |
Ha! I haven't seen that |
Follow-up #5658
The new
cylc set
command has relatively complicated validation requirements. During discussion there, @oliver-sanders suggested validating async scheduler commands server-side before queueing the command, instead of inside the queued command method itself. This enables validation feedback to the CLI and GUI.I have a branch in progress, but it impacts other commands too, so it's better done as a quick-fire follow-up.
The text was updated successfully, but these errors were encountered: