-
Notifications
You must be signed in to change notification settings - Fork 1
Add new field inputs to the spec
#444
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,2 @@ | ||
| EWOKS_KEY = "__ewoks__" | ||
| EWOKS_KEY = "__ewoks_type__" | ||
| EWOKS_FORMAT_KEY = "__ewoks_serialize__" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import sys | ||
| from typing import Any | ||
| from typing import Dict | ||
| from typing import Hashable | ||
| from typing import Literal | ||
| from typing import Optional | ||
|
|
@@ -17,8 +18,12 @@ | |
| from typing import Self | ||
|
|
||
|
|
||
| from jsonschema import Draft7Validator | ||
| from jsonschema.exceptions import SchemaError | ||
| from pydantic import BaseModel | ||
| from pydantic import Field | ||
| from pydantic import ValidationError | ||
| from pydantic import field_validator | ||
| from pydantic import model_validator | ||
|
|
||
| from . import LATEST_VERSION | ||
|
|
@@ -109,6 +114,14 @@ class PpfPortNode(_EwoksBaseNode): | |
| ] | ||
|
|
||
|
|
||
| class EwoksWorkflowInput(BaseModel): | ||
| name: str | ||
| task_identifier: Optional[str] = None | ||
| id: Optional[NodeId] = None | ||
| label: Optional[str] = None | ||
| all_nodes: bool = False | ||
|
|
||
|
|
||
| class EwoksNodeAlias(EwoksNodeAttributes): | ||
| id: NodeId | ||
| node: NodeId | ||
|
|
@@ -123,6 +136,23 @@ class EwoksGraphAttributes(BaseModel): | |
| requirements: Sequence[str] = [] | ||
| input_nodes: Sequence[EwoksNodeAlias] = [] | ||
| output_nodes: Sequence[EwoksNodeAlias] = [] | ||
| inputs: Dict[str, Any] = {} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this dict? It maps what to what? I would expect this to be a Also when defining aliases, we'll need 1-to-many (1 parameter alias, many targets).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok so Draft 2020-12 meta-schema ExamplesJSON schema for a single integermeta_schema = {
"type": "object",
"properties": {
"type": {
"type": "string"
}
}
}schema = {
"type": "number"
}instance = 42JSON schema for a str->int dictionary with keys required value1 and optional value2meta_schema = {
"type": "object",
"properties": {
"type": {
"type": "string"
}
}
}schema = {
"type": "object",
"properties": {
"value1": {
"type": "number"
},
"value2": {
"type": "number"
},
},
"required": ["value"]
}instance = {
"value1": 42,
"value2": 99, # optional
}Ewoks graph attribute
|
||
|
|
||
| @field_validator("inputs") | ||
| @classmethod | ||
| def validate_schema(cls, inputs): | ||
| try: | ||
| Draft7Validator.check_schema(inputs) | ||
| except SchemaError: | ||
| raise | ||
|
woutdenolf marked this conversation as resolved.
Comment on lines
+146
to
+147
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Capture and re-raise. What is the purpose? |
||
|
|
||
| properties = inputs.get("properties", {}) | ||
| for input in properties.values(): | ||
| try: | ||
| EwoksWorkflowInput(**input.get("__ewoks__")) | ||
|
woutdenolf marked this conversation as resolved.
|
||
| except ValidationError: | ||
| raise | ||
|
Comment on lines
+153
to
+154
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Capture and re-raise. What is the purpose? |
||
| return inputs | ||
|
woutdenolf marked this conversation as resolved.
|
||
|
|
||
|
|
||
| class EwoksGraph(BaseModel): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,25 @@ | |
|
|
||
| @graph | ||
| def demo(): | ||
| graph = {"id": "demo", "label": "demo", "schema_version": "1.1"} | ||
| graph = { | ||
| "id": "demo", | ||
| "label": "demo", | ||
| "schema_version": "1.2", | ||
| "inputs": { | ||
| "properties": { | ||
| "task0a": { | ||
| "type": "number", | ||
| "__ewoks__": {"name": "a", "id": "task0"}, | ||
| }, | ||
| "task0b": { | ||
| "type": "number", | ||
| "default": 0, | ||
| "__ewoks__": {"name": "b", "id": 2}, | ||
| }, | ||
|
Comment on lines
+11
to
+20
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using the word "task" or "node" in the parameter name is confusing. I would propose something else. Maybe all-caps for workflow inputs and smallcaps for task inputs. Like global and local variable names in python. Just for the example I mean, this is not something enforced by ewoks. What happens if I have parameter "B" and I want this to be parameter "b" of task1 and task2? Can I do that? Also, "task0" does not have parameter "a" and "b" and there is no "id=2": |
||
| }, | ||
| "required": ["task0a"], | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Required in what sense? The workflow will fail if this parameter is not provided? What happens if I do not provide workflow inputs and I just provide node inputs?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the GUI it can be useful to know whether something is required or not. |
||
| }, | ||
| } | ||
|
|
||
| sumtask = "ewokscore.tests.examples.tasks.sumtask.SumTask" | ||
| sumlist = "ewokscore.tests.examples.tasks.sumlist.SumList" | ||
|
|
@@ -120,7 +138,11 @@ def demo(): | |
| }, | ||
| ] | ||
|
|
||
| taskgraph = {"graph": graph, "links": links, "nodes": nodes} | ||
| taskgraph = { | ||
| "graph": graph, | ||
| "links": links, | ||
| "nodes": nodes, | ||
| } | ||
|
|
||
| expected_results = { | ||
| "task0": {"sum": 3}, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import pydantic | ||
| import pytest | ||
| from jsonschema.exceptions import SchemaError | ||
|
|
||
| from ewokscore import load_graph | ||
| from ewokscore.graph.schema.model import EwoksGraph | ||
|
|
@@ -127,3 +128,45 @@ def test_link_with_datamapping_and_map_all_data(): | |
| match="1 validation error for EwoksGraph\nlinks.0\n", | ||
| ): | ||
| EwoksGraph(**graph_dict) | ||
|
|
||
|
|
||
| def test_input_schema(): | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test is only testing malformed inputs. Perhaps reflect this in the name of the test. |
||
| # Wrong type in schema | ||
| graph_dict = { | ||
| "graph": { | ||
| "id": "required", | ||
| "inputs": {"properties": {"a": {"type": "oo", "__ewoks__": {"name": "a"}}}}, | ||
| }, | ||
| "nodes": [ | ||
| { | ||
| "id": "node1", | ||
| "task_type": "class", | ||
| "task_identifier": "task1", | ||
| }, | ||
| ], | ||
| "links": [], | ||
| } | ||
| with pytest.raises(SchemaError): | ||
| EwoksGraph(**graph_dict) | ||
|
|
||
| # Missing name in __ewoks__ | ||
| graph_dict = { | ||
| "graph": { | ||
| "id": "required", | ||
| "inputs": {"properties": {"a": {"type": "number", "__ewoks__": {}}}}, | ||
| }, | ||
| "nodes": [ | ||
| { | ||
| "id": "node1", | ||
| "task_type": "class", | ||
| "task_identifier": "task1", | ||
| }, | ||
| ], | ||
| "links": [], | ||
| } | ||
|
|
||
| with pytest.raises( | ||
| pydantic.ValidationError, | ||
| match="1 validation error for EwoksGraph\ngraph.inputs.name\n", | ||
| ): | ||
| EwoksGraph(**graph_dict) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Schema version was updated to 1.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI There is a ticket about this: #456