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.

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

Fix issue #282 #404

Merged
merged 2 commits into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[[rec_deploy-replace-undeploy_package_response-cwl]]
[recommendation]
====
[%metadata]
label:: /rec/deploy-replace-undeploy/package/response-cwl

part:: If a process deployed as a <<rc_cwl,CWL>>, implementations SHOULD consider supporting the <<rc_cwl,CWL>> encoding.

====
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[[rec_deploy-replace-undeploy_package_response-ogcapppkg]]
[recommendation]
====
[%metadata]
label:: /rec/deploy-replace-undeploy/package/response-ogcapppkg

part:: If a process was deployed as an <<rc_ogcapppkg,OGC Application Package>>, implementations SHOULD consider supporting the <<rc_ogcapppkg,OGC Application Package>> encoding.
====
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[[req_cwl_package_response-body]]
[requirement]
====
[%metadata]
label:: /req/cwl/package/response-body
part:: A response with HTTP status code `200` SHALL include a body that contains:
* the <<rc_cwl,CWL>> to use to deploy the process, in case the Content-Type used to deploy the process was `application/cwl`.
* the <<rc_ogcapppkg,OGC Application Package>> to use to deploy the process, in case the Content-Type used to deploy the process was `application/ogcapppkg+json`.
Comment on lines +5 to +8
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we consider the possibility that a package deployed one way could provide the representation of another encoding? For example, if deployed using CWL, it would be fairly easy to extract the relevant parts of the document to form the corresponding OGC Application Package format. An Accept header could allow the selection of one, an if conversion is supported, could help make instances more interoperable. If so, sentences mentioning that "if deployed with X, SHOULD support X" would have to be reworded slightly.

Or, do we want to make it explicit that the original format for deployment must be returned (no conversion permitted)? In such case, the Accept header is irrelevant.

Copy link
Member

@jerstlouis jerstlouis Apr 26, 2024

Choose a reason for hiding this comment

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

I vote yes, the implementation should be able to decide which package format it can return back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also favor having the capability to choose between various formats. I agree that CWL (and maybe other encoding. supported in the future) can be converted into an OGC Application Package and vice versa. Nevertheless, I wonder if the Accept header should not be added to the 6.6.1 section, which describes the operation for retrieving the formal description.

Then, in the 6.6 Retrieving the formal description section, we would add another section: Exception specifying what happens if the Accept header is not supported.

The unsupported-media-type exception type is possible if the media type is not supported. However, it's important to note that the exception could be something else if the process cannot be exposed in the desired encoding. For instance, consider a Docker image used to publish a process as an OGC Application Package, then a request /processes/{processId}/package using Accept header set to application/cwl should get back an exception that would help the client application determine that the issue comes from the fact that the conversion is impossible (maybe a type set to http://www.opengis.net/def/exceptions/ogcapi-processes-2/1.0/incompatible-media-type).

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the use the "Accept" header already implied but clause 7.5 "Use of HTTP 1.1"?

Copy link
Contributor

Choose a reason for hiding this comment

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

SWG meeting from April 29th: Add a note that you can request the application package in a format other than originally used using the accept header. Examples: If cwl is explicitly requested and the server cannot handle it that a HTTP 406 should be returned.


====
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[[req_deploy-replace-undeploy_package_get-op]]
[requirement]
====
[%metadata]
label:: /req/deploy-replace-undeploy/package/get-op
part:: For every dynamically added process (path: /processes/{processId}), the server SHALL support the HTTP GET operation at the path `/processes/{processId}/package`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Include POST along with path: /processes/{processId} to avoid ambiguity with GET from core.

part:: The parameter `processId` is each `id` property in the process collection response (JSONPath: `$.processes[*].id`).

====
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[[req_deploy-replace-undeploy_package_response-body]]
[requirement]
====
[%metadata]
label:: /req/deploy-replace-undeploy/package/response-body
part:: A response with HTTP status code `200` SHALL include a body that contains the request body used to deploy or replace the process.
====
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[[req_deploy-replace-undeploy_package_response-success]]
[requirement]
====
[%metadata]
label:: /req/deploy-replace-undeploy/package/response-success
part:: A successful access to the resource SHALL be reported as a response with a HTTP status code `200`.
====
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[[req_ogcapppkg_package_response-body]]
[requirement]
====
[%metadata]
label:: /req/ogcapppkg/package/response-body
part:: A response with HTTP status code `200` SHALL include a body that contains the <<rc_ogcapppkg,OGC Application Package>> to use to deploy the process.
====
Original file line number Diff line number Diff line change
Expand Up @@ -257,3 +257,28 @@ See <<deploy-replace-undeploy-http_status_codes,HTTP status codes>> for general
If the process with the specified identifier does not exist on the server, see requirement /req/core/process-exception/no-such-process from <<OAProc-1>>.

If the process exist but is immutable then <<req_deploy-replace-undeploy_deploy_response-immutable,/req/deploy-replace-undeploy/deploy/response-immutable>> applies.

[[deploy-replace-undeploy-package]]
=== Retrieving the formal description

For every process deployed to the server, it is possible to retrieve its formal description. It corresponds to the request's body used to deploy or replace the process.
Copy link
Contributor

@pvretano pvretano Apr 29, 2024

Choose a reason for hiding this comment

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

Perhaps this wording is more clear? "For every mutable process, it is possible to retrieve the application package that was used to deploy the process."

Copy link
Contributor

Choose a reason for hiding this comment

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

The "mutable" process part raises a good question. What about immutable ones? I have some processes that are immutable, but still represented using CWL. The echo process is a good example of this, as it comes "pre-deployed" with the service.

Copy link
Contributor

Choose a reason for hiding this comment

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

@fmigneault clients know which processes are mutable or not since there is a member in the process description that indicates that. So, clients should not request the "package" of an immutable process since there is no guarantee that the "package" exists. The process may, for example, be compiled right into the executable of the server.
In the event that a client does ask for the package for an immutable process then they should be prepared to get a 404. I am not saying that they will get a 404; if the server has the package or can generate it (as is your case), then the server can offer it up but in the event that a package does not exist the client should be prepared to get a 404. At least that is was I think should happen. Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. That is reasonable.


==== Operation

include::../requirements/deploy-replace-undeploy/package/REQ_get-op.adoc[]

==== Response

===== Overview

include::../requirements/deploy-replace-undeploy/package/REQ_response-success.adoc[]

include::../requirements/deploy-replace-undeploy/package/REQ_response-body.adoc[]

==== Exceptions

See <<deploy-replace-undeploy-http_status_codes,HTTP status codes>> for general guidance.

If the process with the specified identifier does not exist on the server, see requirement /req/core/process-exception/no-such-process from <<OAProc-1>>.

If the process exist but is immutable then <<req_deploy-replace-undeploy_deploy_response-immutable,/req/deploy-replace-undeploy/deploy/response-immutable>> applies.
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,9 @@ include::../requirements/ogcapppkg/deploy/REQ_body.adoc[]
==== OGC Application Package body

include::../requirements/ogcapppkg/replace/REQ_body.adoc[]

=== Formal description

==== OGC Application Package content

include::../requirements/ogcapppkg/package/REQ_response-body.adoc[]
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,8 @@ include::../requirements/cwl/deploy/REQ_exception-workflow-not-found.adoc[]

include::../requirements/cwl/replace/REQ_body.adoc[]

=== Formal description

==== CWL content

include::../requirements/cwl/package/REQ_response-body.adoc[]
25 changes: 25 additions & 0 deletions openapi/paths/processes-dru/pPackage.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
get:
summary: retrieve the formal description used to deploy a process
description: |
Access the formal description that can be used to deploy a process on an OGC API - Processes Server Instance.
operationId: getPackage
tags:
- Package
responses:
200:
description: The formal used to deploy a process
content:
application/ogcapppkg+json:
schema:
$ref: "../../schemas/processes-dru/ogcapppkg.yaml"
application/cwl:
schema:
$ref: "https://raw.githubusercontent.com/common-workflow-language/cwl-v1.2/1.2.1_proposed/json-schema/cwl.yaml"
application/cwl+json:
schema:
$ref: "https://raw.githubusercontent.com/common-workflow-language/cwl-v1.2/1.2.1_proposed/json-schema/cwl.yaml"
application/cwl+yaml:
schema:
$ref: "https://raw.githubusercontent.com/common-workflow-language/cwl-v1.2/1.2.1_proposed/json-schema/cwl.yaml"
Copy link
Contributor

Choose a reason for hiding this comment

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

The URI is not valid anymore (to fix in other parts of the spec as well).
I already flagged the issue here: common-workflow-language/cwl-v1.2#278 (comment)

Waiting for their feedback for an official location.

404:
$ref: "../../responses/common-core/rNotFound.yaml"
Loading