Add support for multi deployment using workflow cwl by Nazim-crim · Pull Request #968 · crim-ca/weaver

Nazim-crim · 2026-05-22T17:39:15Z

codecov · 2026-05-22T20:08:05Z

Codecov Report

❌ Patch coverage is 94.55446% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.30%. Comparing base (1b9edbb) to head (1ddf15b).
⚠️ Report is 6 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
weaver/processes/utils.py	94.47%	2 Missing and 9 partials ⚠️

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.
📢 Have feedback on the report? Share it here.

…lti part deploy from cwl list, add test

fmigneault

Review still in progress. Posting pending comments to stage in the meantime.

fmigneault · 2026-06-03T14:48:31Z

+        # Since there's no workflow, first tool should be the main process
+        main_id = result["processSummary"]["id"]
+        assert main_id in [f"{test_id}-tool-1", f"{test_id}-tool-2"]


I think this operation should instead be 400.

Based on https://www.commonwl.org/v1.2/CommandLineTool.html#Packed_documents

If the reference to the packed document does not include a fragment identifier, the runner must choose the top-level process object as the entry point. If there is no top-level process object (as in the case of $graph) then the runner must choose the process object with an id of #main. If there is no #main object, the runner must return an error.

Therefore, CWL will execute by default only the first one, which makes the rest of the definition irrelevant. To avoid proliferation / misinterpretation of partial definitions, I think Weaver should detect and refuse this case early on.

fmigneault · 2026-06-03T14:55:23Z

-    return package
+    # type: (CWL) -> Union[CWL, List[CWL]]
+    """
+    Resolve CWL $graph into deployable packages.


Missing :term: and code quotes for fragments found inside CWL, Python keywords, etc.

Same for other docstrings.

fmigneault · 2026-06-03T14:59:29Z

            If an embedded ``executionUnit`` containing the :term:`CWL` is desired, provide ``body={}`` explicitly.
+            Can also be a list of :term:`CWL` definitions (dicts, strings, or file paths) for multi-process
+            deployment, which will be combined into a multipart request. When deploying multiple CWL files,
+            the workflow (if present) should be last in the list, and all tools should come before it.


I think the "last in the list" requirement is a side effect of the order the code uses to do validation, which should not impact the deployment content structure. Or it this just an artifact of earlier definition since the code has a sorting/resolution function.

As long as there is 1 Workflow, I think it could be anywhere.

Since we only handle 1 workflow (for now at least), the Content-ID or id reference in the CWL could be used to identify the relevant one, as per https://www.commonwl.org/v1.2/CommandLineTool.html#Packed_documents.

fmigneault · 2026-06-03T18:14:34Z

+# =============================================================================
+# Multipart Deployment Parsing Tests
+# =============================================================================
+# These tests directly test the internal multipart parsing functions to cover
+# branches that are difficult to reach through integration tests.


Move under common class or remove comment
Consider adding a @pytest.mark.multipart marker on class or each test individually.

fmigneault · 2026-06-03T18:16:42Z

+        assert result.body["id"] == test_id
+        assert result.body["processDescriptionURL"]
+
+    def test_deploy_multi_cwl_tools_only(self):


Error according to previous comment.

fmigneault · 2026-06-03T18:20:37Z

+def test_get_multipart_content_with_bytes():
+    """
+    Test _get_multipart_content with bytes input.
+    """
+    content = b"test content"
+    result = _get_multipart_content(content, request=None)
+    assert result == b"test content"
+    assert isinstance(result, bytes)


This and other ones similar below should be combined under a single test function with @pytest.mark.parametrized to make supported combinations easier to interpret all at once.

fmigneault · 2026-06-03T18:22:36Z

+    if not isinstance(part_data, dict):
+        return process_description
+
+    if 'class' in part_data and part_data['class'] in ['CommandLineTool', 'Workflow', 'ExpressionTool']:


inconsistent string, use double quotes, here and below

fmigneault · 2026-06-03T18:33:25Z

+def parse_multipart_deploy(content, content_type, request=None):
+    # type: (Union[str, bytes], str, Optional[AnyRequestType]) -> Tuple[List[CWL], Optional[JSON]]
+    """
+    Parse multipart/mixed or multipart/related deployment content.


A lot of ignored code paths in this function (# pragma: no cover). If those are considered supported error-handling/robustness cases, they should be tested properly.

fmigneault · 2026-06-03T19:58:46Z

+        # Check if content should be fetched from Content-Location
+        # Only fetch if part body is empty AND Content-Location looks like a URL
+        if content_location and (not part_content or not part_content.strip()):


Move this if block in a separate function to make the main one easier to interpret.

fmigneault · 2026-06-03T19:59:04Z

+    # 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
+            )


Move this in a separate function

fmigneault · 2026-06-03T20:07:55Z

+def parse_multipart_deploy(content, content_type, request=None):
+    # type: (Union[str, bytes], str, Optional[AnyRequestType]) -> Tuple[List[CWL], Optional[JSON]]


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

fmigneault · 2026-06-03T20:11:35Z

+        # Verify child tools were deployed
+        if "deployedProcesses" in result:
+            assert f"{test_id}-echo-tool" in result["deployedProcesses"]
+            assert f"{test_id}-cat-tool" in result["deployedProcesses"]


not applicable

fmigneault · 2026-06-03T20:12:01Z

+        # Verify child tools were deployed
+        if "deployedProcesses" in result:
+            assert f"{test_id}-echo-tool" in result["deployedProcesses"]
+            assert f"{test_id}-cat-tool" in result["deployedProcesses"]
+
+            # Verify we can retrieve the child tools
+            desc = self.get_process_description(f"{test_id}-echo-tool", schema=ProcessSchema.OLD)
+            assert desc["process"]["id"] == f"{test_id}-echo-tool"
+            pkg = self.get_application_package(f"{test_id}-echo-tool")
+            assert pkg["class"] == "CommandLineTool"
+
+            desc = self.get_process_description(f"{test_id}-cat-tool", schema=ProcessSchema.OLD)
+            assert desc["process"]["id"] == f"{test_id}-cat-tool"
+            pkg = self.get_application_package(f"{test_id}-cat-tool")
+            assert pkg["class"] == "CommandLineTool"


not applicable

only main proces should be checked then ?

My bad, I over-selected lines. The deployedProcesses part is not applicable, since it is never returned. Only the "main workflow" process is returned in the response. For validation of the procedure though, yes, check that each process individually was deployed correctly.

Add support for multi deployment using workflow cwl

fa9584b

Nazim-crim self-assigned this May 22, 2026

github-actions Bot added ci/tests Tests of the package and features ci/doc Issue related to documentation of the package process/wps3 Issue related to WPS 3.x (REST-JSON) processes support feature/oas Issues related to OpenAPI specifications. labels May 22, 2026

Nazim-crim added 3 commits May 22, 2026 14:19

Add multi format support and test

6bf58e9

Fix docstring

9a28cec

Fix mock for test

6f7539a

Nazim-crim added 5 commits May 27, 2026 11:07

Fix multi part and add test coverage

a71e0db

Add content location fetch in multipart deploy

9923511

Fix check

1d158b7

Merge branch 'master' into multipart-deployment

eb9a5bf

Add cli support for multiple cwl file, add util function to create mu…

e82fbc0

…lti part deploy from cwl list, add test

github-actions Bot added the feature/cli Issues or features related to CLI operations. label May 29, 2026

Nazim-crim added 9 commits June 1, 2026 10:35

Move type checking, fix test cleanup

b2d187d

Add check for test case

c2b6d5e

Merge branch 'master' into multipart-deployment

d13ea2b

Removed unused else, add pragma no cover to defensive check

cd9bff0

Add pragma no cover to try catch

d2b22eb

Increase coverage util test

6d38595

Fix pylint

2bd9e86

Merge branch 'master' into multipart-deployment

77028da

Removed unused typing

2cf0334

Nazim-crim requested a review from fmigneault June 2, 2026 17:52

Nazim-crim added 4 commits June 2, 2026 13:55

Removed unused comment

1ddf15b

Merge branch 'master' into multipart-deployment

f9fa7b1

Fix typing

c96f359

add test case, update change log

e0290c3

Nazim-crim added 3 commits June 3, 2026 10:46

Fix lint

204436b

add missing line

c85f626

Merge branch 'master' into multipart-deployment

04b465c

fmigneault requested changes Jun 3, 2026

View reviewed changes

Nazim-crim added 4 commits June 3, 2026 15:03

Fix doc consistency, add test for cli multi deployment

0d2edc4

Update change log

1bda630

Fix line too long lint

b112aad

if check for multi deploy

4975352

fmigneault requested changes Jun 3, 2026

View reviewed changes

		def parse_multipart_deploy(content, content_type, request=None):
		# type: (Union[str, bytes], str, Optional[AnyRequestType]) -> Tuple[List[CWL], Optional[JSON]]

Conversation

Nazim-crim commented May 22, 2026 • edited by fmigneault Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

fmigneault left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Nazim-crim commented May 22, 2026 •

edited by fmigneault

Loading

codecov Bot commented May 22, 2026 •

edited

Loading