Add support for multi deployment using workflow cwl#968
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #968 +/- ##
==========================================
+ Coverage 88.22% 88.30% +0.07%
==========================================
Files 88 88
Lines 20569 20755 +186
Branches 2702 2744 +42
==========================================
+ Hits 18148 18327 +179
- Misses 1733 1735 +2
- Partials 688 693 +5 ☔ View full report in Codecov by Sentry. |
fmigneault
left a comment
There was a problem hiding this comment.
Review still in progress. Posting pending comments to stage in the meantime.
| # Reorder if root workflow specified and validate it | ||
| if root_workflow_cid and root_workflow_cid in parts_by_cid: | ||
| root_pkg = parts_by_cid[root_workflow_cid] | ||
| # Validate that the root is actually a Workflow (per RFC 5621 and multipart/related requirements) | ||
| root_class = root_pkg.get("class", "") | ||
| if root_class != "Workflow": | ||
| raise HTTPBadRequest(json={ | ||
| "title": "Invalid root workflow reference", | ||
| "description": ( | ||
| f"The 'start' parameter references a CWL with class '{root_class}', " | ||
| "but only 'Workflow' is permitted as root document in multipart/related." | ||
| ), | ||
| "cause": {"Content-ID": root_workflow_cid, "class": root_class} | ||
| }) | ||
| cwl_packages = [pkg for pkg in cwl_packages if pkg is not root_pkg] | ||
| cwl_packages.append(root_pkg) | ||
| elif not root_workflow_cid and cwl_packages: | ||
| # No explicit start parameter: validate first element is a Workflow (RFC 5621 §7 default) | ||
| first_pkg = cwl_packages[0] | ||
| first_class = first_pkg.get("class", "") | ||
| if first_class and first_class != "Workflow": | ||
| LOGGER.warning( | ||
| "No 'start' parameter provided in multipart/related. First element has class '%s' " | ||
| "but 'Workflow' is recommended for root document. Proceeding with deployment.", | ||
| first_class | ||
| ) |
There was a problem hiding this comment.
Move this in a separate function
There was a problem hiding this comment.
moved to _validate_and_reorder_multipart_workflow , should it split into two or its fine like this
| def parse_multipart_deploy(content, content_type, request=None): | ||
| # type: (Union[str, bytes], str, Optional[AnyRequestType]) -> Tuple[List[CWL], Optional[JSON]] |
There was a problem hiding this comment.
Please refactor in smaller chunks.
In the long run, I plan to support multipart "Deploy+Execute" (like this #834 (comment)).
Therefore, I will need to inject other intermediate/diverging steps, and would like to reuse these functions such as _classify_multipart_part. I would need to have the steps better split out:
- parse multipart => [parts]
- for part => interpret part
- for now, you consider only CWL / process description as you currently do
- I can latch on these functions filter "Execute" later
- filter / sort parts in relevant way
Uh oh!
There was an error while loading. Please reload this page.