launch: include validation warnings in returned message #611
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #611 +/- ##
==========================================
- Coverage 59.31% 59.25% -0.07%
==========================================
Files 32 32
Lines 3230 3235 +5
==========================================
+ Hits 1916 1917 +1
- Misses 1314 1318 +4
|
giuseppe-steduto
force-pushed
the
reana-yaml-allow-unexpected-props
branch
from
56b23ac
to
9215f28
Compare
reana_server/rest/launch.py
Outdated
| response_data = { | ||
| "workflow_id": workflow.id_, | ||
| "workflow_name": workflow.name, | ||
| "message": "The workflow has been successfully submitted.", | ||
| "message": message, |
There was a problem hiding this comment.
Maybe we can serialise the warnings in a new validation_warnings (name can be changed) field of the reply, so that we can keep it structured? Otherwise it is impossible client/frontend-side to know whether there were warnings when validating the specification without parsing the message.
Something like
{
"...": "...",
"validation_warnings": {
"additional_properties": ["x", "y", "z"]
}
}We can discuss this in IRL, too.
giuseppe-steduto
force-pushed
the
reana-yaml-allow-unexpected-props
branch
2 times, most recently
from
ca901bd
to
a939da8
Compare
giuseppe-steduto
force-pushed
the
reana-yaml-allow-unexpected-props
branch
2 times, most recently
from
9a78d6b
to
31e1ac7
Compare
CHANGES.rst
Outdated
| @@ -4,6 +4,7 @@ Changes | |||
| Version 0.9.1 (UNRELEASED) | |||
| -------------------------- | |||
|
|
|||
| - Changes OpenAPI specification to include the optional ``validation_warnings`` key in the response of the ``launch`` endpoint. | |||
There was a problem hiding this comment.
Given that not only the OpenAPI spec has changed, but also what the endpoint returns, I would say something like Changes ``launch`` endpoint to also include the warnings of the validation of the workflow specification. What do you think?
| description: >- | ||
| Dictionary of validation warnings, if any. Each | ||
| key is a property that was not correctly validated. | ||
| type: object |
There was a problem hiding this comment.
What do you think about specifying this more, so that it is easily usable also in r-client-go? With the current specification, validation_warnings is a interface{} (note that currently we do not use the launch endpoint in the clients, but I think it's still a good idea to have precise OpenAPI specs)
reana_server/validation.py
Outdated
| @@ -107,7 +107,7 @@ def validate_inputs(reana_yaml: Dict) -> None: | |||
| ) | |||
|
|
|||
|
|
|||
| def validate_workflow(reana_yaml: Dict, input_parameters: Dict) -> None: | |||
| def validate_workflow(reana_yaml: Dict, input_parameters: Dict) -> List[Exception]: | |||
There was a problem hiding this comment.
I think this should be Dict[...] and not List[Exception]
giuseppe-steduto
force-pushed
the
reana-yaml-allow-unexpected-props
branch
2 times, most recently
from
1375649
to
8c490d8
Compare
In case some warnings are issued when launching a workflow, the `launch` endpoint will now include them in a new `validation_warnings` key. Closes reanahub/reana-client#660.
giuseppe-steduto
force-pushed
the
reana-yaml-allow-unexpected-props
branch
from
8c490d8
to
d5d9438
Compare
In case some warnings are issued when launching a workflow, the
launchendpoint will now include them in the returnedmessage.Closes reanahub/reana-client#660.