Hexa 1134 dhis2 perameter validation #225
Conversation
nazarfil
force-pushed
the
HEXA-1134-dhis2-perameter-validation
branch
from
f4ffc9d to
322fd7f
Compare
nazarfil
force-pushed
the
HEXA-1134-dhis2-perameter-validation
branch
2 times, most recently
from
be3ef0d to
fb2956a
Compare
nazarfil
requested review from
qgerome and
YolanFery
and removed request for
qgerome
| @@ -159,6 +161,8 @@ def get_pipeline(pipeline_path: Path) -> Pipeline: | |||
| parameter = Parameter(type=type_class.expected_type, **parameter_args) | |||
| pipelines_parameters.append(parameter) | |||
|
|
|||
| validate_parameters_with_connection(pipelines_parameters) | |||
There was a problem hiding this comment.
I was thinking about what could we validate in SDK, and except verifying the link between a parameter with connection field and another parameter with related connection type I couldn't come up with anything else to check on.
Check if such connection if defined in database is already done.
There was a problem hiding this comment.
From what I understand, checking that there is a link is enough. My suggestion above might help a little to make it more accurate
| "connectionSlug": connection_slug, | ||
| }, | ||
| ) | ||
| response["data"]["connectionBySlug"]["status"] == "UP" |
There was a problem hiding this comment.
On the pipeline run we could check in case of one of the parameters is DHIS2 connection if the server is up. But I am not sure it is adding much value in cli - wdyt?
nazarfil
force-pushed
the
HEXA-1134-dhis2-perameter-validation
branch
from
fb2956a to
1d2b736
Compare
tests/test_ast.py
Outdated
| def test_pipeline_with_widget_without_connection(self): | ||
| """The file contains a @pipeline decorator and a @parameter decorator with a widget parameter field.""" | ||
| with tempfile.TemporaryDirectory() as tmpdirname: | ||
| with open(f"{tmpdirname}/pipeline.py", "w") as f: | ||
| f.write( | ||
| "\n".join( | ||
| [ | ||
| "from openhexa.sdk.pipelines import pipeline, parameter", | ||
| "", | ||
| "@parameter('test_field_for_wdiget', name='Widget Param', type=str, widget='custom_picker' help='Param help')", | ||
| "@pipeline('test', 'Test pipeline')", | ||
| "def test_pipeline():", | ||
| " pass", | ||
| "", | ||
| ] | ||
| ) | ||
| ) |
There was a problem hiding this comment.
This test does nothing, right ?
| """Validate the provided connection parameters if they relate to existing connection parameter.""" | ||
| for parameter in parameters: | ||
| if parameter.connection is not None: | ||
| if not any(p.code == parameter.connection for p in parameters): |
There was a problem hiding this comment.
We might want to use only the parameters with a connection type (like type=DHIS2Connection), it's a bit more accurate than searching in the full list of parameters
| @@ -159,6 +161,8 @@ def get_pipeline(pipeline_path: Path) -> Pipeline: | |||
| parameter = Parameter(type=type_class.expected_type, **parameter_args) | |||
| pipelines_parameters.append(parameter) | |||
|
|
|||
| validate_parameters_with_connection(pipelines_parameters) | |||
There was a problem hiding this comment.
From what I understand, checking that there is a link is enough. My suggestion above might help a little to make it more accurate
adds
widgetfield to parametersadds
connectionfield to parametersadds check on connection between parameters
removes remainings of parameter specs
https://bluesquare.atlassian.net/browse/HEXA-1134