Connection types #480
Connection types #480
Conversation
| return EcoscopeWorkflowsNamedConnection_EarthRanger | ||
|
|
||
|
|
||
| @task | ||
| def set_smart_connection( | ||
| EcoscopeWorkflowsNamedConnection_SMART: Annotated[ | ||
| str, | ||
| Field(description="A named connection.", title="Name"), | ||
| ], | ||
| ) -> str: | ||
| return EcoscopeWorkflowsNamedConnection_SMART | ||
|
|
||
|
|
||
| @task | ||
| def set_gee_connection( | ||
| EcoscopeWorkflowsNamedConnection_GoogleEarthEngine: Annotated[ | ||
| str, | ||
| Field(description="A named connection.", title="Name"), | ||
| ], | ||
| ) -> str: | ||
| return EcoscopeWorkflowsNamedConnection_GoogleEarthEngine |
There was a problem hiding this comment.
Cool. Generally I am good with this and don't want to create unnecessary blockers but I did have one thought I'd like to discuss if you have a moment.
Apart from this smart / gee stuff, I was wondering if I made a bit of a strategic misstep in how these values are being substituted / parsed on the backend.
Rather than matching against parameter name, I wonder if should be matching against some type (basically, a string alias), which would make the rjsf post-processing be "just" a matter of replacing a $def?
There was a problem hiding this comment.
I think we should consider adopting the pattern already used for the clients to resolve the connection from a string, rather than an object. This to me seems more readable from the RJSF layer (less nested), and also the python layer (since we can use a "regular" snake case parameter name).
Then the enum substitution on the server side can be changed to be based on the $def of these types (rather than the recursive search for the parameter names). This is more scalable and stable (i.e. $def-based find-and-replace, rather than reserved parameter name find-and-replace) as we look at other types of dynamic enum substitution going forward.
Finally, we will also need to change the parsing for the connection clients to be aware that they will be passed an EarthRangerConnection (and not a string).
examples/events/test-cases.yaml
Outdated
| EcoscopeWorkflowsNamedConnection: | ||
| name: "mep_dev" |
There was a problem hiding this comment.
| EcoscopeWorkflowsNamedConnection: | |
| name: "mep_dev" | |
| connection: "mep_dev" |
| @task | ||
| def set_er_connection( | ||
| EcoscopeWorkflowsNamedConnection: Annotated[ | ||
| EarthRangerConnection, | ||
| Field(description="A named connection.", title="Name"), | ||
| ], | ||
| ) -> EarthRangerConnection: | ||
| return EcoscopeWorkflowsNamedConnection | ||
|
|
||
|
|
||
| @task | ||
| def set_smart_connection( | ||
| EcoscopeWorkflowsNamedConnection: Annotated[ | ||
| SMARTConnection, | ||
| Field(description="A named connection.", title="Name"), | ||
| ], | ||
| ) -> SMARTConnection: | ||
| return EcoscopeWorkflowsNamedConnection | ||
|
|
||
|
|
There was a problem hiding this comment.
| @task | |
| def set_er_connection( | |
| EcoscopeWorkflowsNamedConnection: Annotated[ | |
| EarthRangerConnection, | |
| Field(description="A named connection.", title="Name"), | |
| ], | |
| ) -> EarthRangerConnection: | |
| return EcoscopeWorkflowsNamedConnection | |
| @task | |
| def set_smart_connection( | |
| EcoscopeWorkflowsNamedConnection: Annotated[ | |
| SMARTConnection, | |
| Field(description="A named connection.", title="Name"), | |
| ], | |
| ) -> SMARTConnection: | |
| return EcoscopeWorkflowsNamedConnection | |
| @task | |
| def set_er_connection( | |
| connection: Annotated[ | |
| EarthRangerConnection, | |
| Field(description="A named connection.", title="Name"), | |
| ], | |
| ) -> EarthRangerConnection: | |
| return EcoscopeWorkflowsNamedConnection | |
| @task | |
| def set_smart_connection( | |
| connection: Annotated[ | |
| SMARTConnection, | |
| Field(description="A named connection.", title="Name"), | |
| ], | |
| ) -> SMARTConnection: | |
| return EcoscopeWorkflowsNamedConnection | |
I think we should consider adapting this pattern to allow for passing this value as a string, rather than an object
There was a problem hiding this comment.
I'm wondering, since these defs are now specific to ER/SMART/GEE, does it make sense to move them into ext-ecoscope?
There was a problem hiding this comment.
Hmm interesting point 🤔
Well I think they do meet the criteria of not importing anything aside from builtins, so aesthetically I see your point, but functionally I'm not as concerned. Then again, I could imagine other extensions wanting to set and use these connections ... so maybe we just leave them here?
* pull in alex's params.json artifact from #480 * instead of exec, use params.json to generate params_sha256 * auto: recompile all from 4bb3796 * don't need title for json artifact * auto: recompile all from f163f00 * try for less re-computation in package artifacts creation * title is required * auto: recompile all from 4feecb0 * account for param.json in from_disk * auto: recompile all from e7ebcad * oops flip/flopped the models * auto: recompile all from 8e66eb5 * refine cleaning of params * auto: recompile all from 514031c * more jsonschema WIP * more properties * check nested * omit unused defs * mypy * oops items * auto: recompile all from 8b9c9ec * auto: recompile all from c270eb6 --------- Co-authored-by: ecoscope-elebot <[email protected]>
closes #479