Skip to content

Add support for multi deployment using workflow cwl#968

Open
Nazim-crim wants to merge 60 commits into
masterfrom
multipart-deployment
Open

Add support for multi deployment using workflow cwl#968
Nazim-crim wants to merge 60 commits into
masterfrom
multipart-deployment

Conversation

@Nazim-crim

@Nazim-crim Nazim-crim commented May 22, 2026

Copy link
Copy Markdown
Contributor

@Nazim-crim Nazim-crim self-assigned this May 22, 2026
@github-actions 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
@codecov

codecov Bot commented May 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.68340% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.38%. Comparing base (ea311a3) to head (31dcbdd).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
weaver/processes/utils.py 97.37% 0 Missing and 6 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #968      +/-   ##
==========================================
+ Coverage   88.23%   88.38%   +0.15%     
==========================================
  Files          88       88              
  Lines       20573    20808     +235     
  Branches     2702     2753      +51     
==========================================
+ Hits        18152    18391     +239     
+ Misses       1733     1729       -4     
  Partials      688      688              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@github-actions github-actions Bot added the feature/cli Issues or features related to CLI operations. label May 29, 2026
@Nazim-crim Nazim-crim requested a review from fmigneault June 2, 2026 17:52
@Nazim-crim Nazim-crim marked this pull request as ready for review June 12, 2026 18:56
@github-actions github-actions Bot added the ci/operations Related to CI operations (actions, execution, install, builds, etc.) label Jun 16, 2026
@Nazim-crim Nazim-crim requested a review from fmigneault June 16, 2026 19:18

@fmigneault fmigneault left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for the features. We just need to document the things that were implemented.

Comment thread setup.cfg Outdated
Comment thread CHANGES.rst

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to properly reflect the added functionalities, the corresponding /rec, /per, /req and /conf elements defined in https://github.com/opengeospatial/ogcapi-processes/pull/588/changes#diff-6dbd783db0ade3c408ddc75d4271f2af2478056344e081a02f48a495fdde4716 need to be added to the weaver/wps_restapi/api.py conformance list.

The should use the ogcapi_proc_part2 prefix. To be placed where other similar definitions about ../cwl/.. can be found, in the corresponding order to make it easier to find/double-check them.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only see the /rec , is there something else to add ?

 f"{ogcapi_proc_part2}/rec/deploy-replace-undeploy/deploy-body-cwl-multipart",
f"{ogcapi_proc_part2}/rec/deploy-replace-undeploy/replace-body-cwl-multipart
```",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm having issue understanding the changes / what needs to be taken from this pr

I see this new one

/per/cwl/multipart-cwl-body

and that deploy-body-cwl-multipart has been remove

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The 2 previous definitions have been combined into a single one (note the /rec -> /per change).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So its only this to be added ?
/per/cwl/multipart-cwl-body

Since the other are regarding the w parameter

Comment thread weaver/cli.py Outdated
Comment thread weaver/cli.py
Comment thread weaver/cli.py Outdated
Comment thread weaver/processes/utils.py Outdated
Comment thread weaver/processes/utils.py Outdated
Comment thread weaver/processes/utils.py Outdated
Comment on lines +1542 to +1543
assert resp.status_code == 400, \
f"Expected 400 Bad Request for $graph without #main entry point, got {resp.status_code}: {resp.json}"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have a few extra tests for error prone cases:

  1. With #main set on a CommandLineTool where more than one CommandLineTool is provided and no Workflow is present.
  2. With #main set on a CommandLineTool while Workflow has something else.
  3. With #main set on both CommandLineTool and Workflow.

In each case, those combination should create a similar error.

@Nazim-crim Nazim-crim Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When its a workflow there should always be a #main id ?, only in the case when using with graph right *

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is only 1 workflow, it is safe to assume it is the main if no #main is provided. It should be provided if there is more than one, and it must not be on a non workflow item. Should apply to any dict/graph/multipart variant when dealing with the list of resolved CWLs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We dont allow more than 1 workflow in this case

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We still need to check if there are more than one occurences of #main, such as for case (3) since the workflow > 1: raise condition will not be triggered.

Comment thread tests/processes/test_utils.py
@Nazim-crim Nazim-crim requested a review from fmigneault June 22, 2026 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci/doc Issue related to documentation of the package ci/operations Related to CI operations (actions, execution, install, builds, etc.) ci/tests Tests of the package and features feature/cli Issues or features related to CLI operations. feature/oas Issues related to OpenAPI specifications. process/wps3 Issue related to WPS 3.x (REST-JSON) processes support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support multi-CWL file deployment Support Multipart CWL Deployment Support embedded step definition in CWL

2 participants