Skip to content
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.

Sign up for GitHub

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

Jump to bottom

handle CWL Directory type #474

Merged
merged 104 commits into from
Nov 23, 2022
Merged

handle CWL Directory type #474

merged 104 commits into from
Nov 23, 2022

Conversation

fmigneault
Copy link
Collaborator

Changes

  • Support CWL Directory type as application/directory Media-Type

References

@fmigneault fmigneault self-assigned this Oct 12, 2022
@github-actions github-actions bot added ci/doc Issue related to documentation of the package ci/tests Tests of the package and features labels Oct 12, 2022
@github-actions github-actions bot added feature/oas Issues related to OpenAPI specifications. process/wps3 Issue related to WPS 3.x (REST-JSON) processes support feature/CWL Issue related to CWL support labels Oct 12, 2022
@github-actions github-actions bot added ci/operations Related to CI operations (actions, execution, install, builds, etc.) feature/cli Issues or features related to CLI operations. feature/opensearch Issue related to OpenSearch functionalities. process/builtin Issue related to builtin application processes labels Oct 15, 2022
@github-actions github-actions bot added feature/docker Issue related to Docker application package execution. process/workflow Related to a Workflow process. labels Oct 19, 2022
@francisPLT
Copy link
Contributor

Read through the code as discussed, everything looks good, except that the last commit (1b0e14c) seems to have broken something

I get this error when running tests of trying to launch the application locally.

weaver.wps_restapi.colander_extras.SchemaNodeTypeError: Keyword schema 'CWLHintsItem' of type 'oneOf' specification with 'discriminator' attempts to refer to duplicate example values 'InitialWorkDirRequirement' between 'InitialWorkDirRequirementClass' and 'InlineJavascriptRequirementClass'

Everything worked perfectly using the previous commit, so as soon as this issue is resolved, this PR is approved.

francisPLT
francisPLT previously approved these changes Nov 22, 2022
View reviewed changes
Copy link
Contributor

@francisPLT francisPLT left a comment

Choose a reason for hiding this comment

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

Previously mentioned problem is now fixed.

Feel free to re-request a review after the final failing test is fixed, but I'm approving it for now so you can merge when ready

francisPLT
francisPLT previously approved these changes Nov 23, 2022
View reviewed changes
@fmigneault
Copy link
Collaborator Author

Failing test in bad4cf9 was a one-off HTTP connection error.
Following commits add more test cases and validations.

@fmigneault fmigneault merged commit fb6e9c8 into master Nov 23, 2022
@fmigneault fmigneault deleted the dir-type branch November 23, 2022 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@francisPLT francisPLT francisPLT left review comments

@ChaamC ChaamC Awaiting requested review from ChaamC

Assignees

@fmigneault fmigneault

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/CWL Issue related to CWL support feature/docker Issue related to Docker application package execution. feature/oas Issues related to OpenAPI specifications. feature/opensearch Issue related to OpenSearch functionalities. feature/providers Issue related to providers convertion to WPS-REST processes. process/builtin Issue related to builtin application processes process/esgf-cwt Issue related to ESGF-CWT components. process/workflow Related to a Workflow process. process/wps3 Issue related to WPS 3.x (REST-JSON) processes support
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support CWL Directory type
2 participants
@fmigneault @francisPLT