Enhancement/improve subprocess handling #187
Conversation
…askState.XXX. Should all work as expected now.
…he top level rather than storing them on the task spec
|
Tried opening up an existing workflow, and got an error in BpmnWorkflow on line 79. It's a key error trying to look in the sub-processes. It happens as soon as it gets to the first Call Activity here: |
| @@ -134,7 +129,7 @@ def add_bpmn_files(self, filenames): | |||
| finally: | |||
| f.close() | |||
|
|
|||
| def add_bpmn_xml(self, bpmn, svg=None, filename=None): | |||
There was a problem hiding this comment.
Why are removing the svg, is tracking that a problem?
There was a problem hiding this comment.
I removed it because it was just being passed around but not used for anything within Spiff. The only place it was referenced was by the packager, which is also not used by anything except the old (deprecated) serializer (I wanted to remove that as well, but don't want to go to the trouble of getting the deprecated serializer to work properly without it).
|
|
||
| CAMUNDA_MODEL_NS = 'http://camunda.org/schema/1.0/bpmn' | ||
|
|
||
| class NodeParser: |
There was a problem hiding this comment.
This here is a lovely lovely piece of work.
There was a problem hiding this comment.
My vision for the parser is to actually not have a task parsers as they're currently implemented but node parsers. It would be reasonable for users to want to write other parsers besides task parsers, so it would be better if our parser wasn't based on tasks, but XML nodes. Our parser ought to have the ability to call a custom parser for any XML element. This change doesn't get us there 100% but it's a step in that direction.
|
|
||
| def _detect_multiinstance(self): | ||
|
|
||
| # get special task decorators from XML |
There was a problem hiding this comment.
This delete deserves a very fine cookie.
| @@ -0,0 +1,54 @@ | |||
| from copy import deepcopy | |||
|
|
|||
| def version_1_0_to_1_1(old): | |||
There was a problem hiding this comment.
Very nicely done. Can't think of a better way to handle it.
| @@ -123,6 +125,8 @@ def serialize_json(self, workflow, use_gzip=False): | |||
| def deserialize_json(self, serialization, read_only=False, use_gzip=False): | |||
| dct = json.loads(gzip.decompress(serialization)) if use_gzip else json.loads(serialization) | |||
| version = dct.pop('serializer_version') | |||
| if version in MIGRATIONS: | |||
There was a problem hiding this comment.
I guess in the future this might get more complicated, as you would run 1 to 1.1 then 1.1 to 1.2 .... But seems overkill to fix that at the moment.
There was a problem hiding this comment.
I thought about this a fair amount, but ended up going with the simplest solution, because a complicated solution that attempts to anticipate issues might not anticipate the right ones and then end up merely being complicated without solving any actual problems.
I'm not sure 1.0 -> 1.1, 1.1 -> 1.2, etc is that bad (though we should probably turn the migrations file into it's own package if we accumulate a lot of changes). We can automate it pretty easily for people who are not customizing the serializer, and it's better to have discrete pieces of code that anyone who does can use or extend.
|
|
||
| return element_var_data | ||
|
|
||
| def _on_complete_hook(self, my_task): | ||
| # do special stuff for non-user tasks | ||
| self._handle_special_cases(my_task) | ||
|
|
…sing a validation exception. get_version will now accept a dict or a json string and do the right thing. when creating a workflow from a dictionary - it will check the version_key and upgrade if needed (this was just happening when you called deserialize_json, now it happens in both cases)
…sartography/SpiffWorkflow into enhancement/improve-subprocess-handling
|
Kudos, SonarCloud Quality Gate passed! |
This PR changes the way subworkflows are parsed and nandled internally.
This is a draft PR because I still need to update the documentation.