Skip to content

Commit

Permalink
CLI fixes and default handling of visiblity flag
Browse files Browse the repository at this point in the history
  • Loading branch information
fmigneault committed Sep 16, 2023
1 parent 366823a commit 9f8acab
Show file tree
Hide file tree
Showing 7 changed files with 128 additions and 13 deletions.
11 changes: 11 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,13 @@ Changes:
`Application Package` definition generated for the specific `Provider`'s `Process` definition.
- Add `CLI` ``package`` operation to request the remote `Provider` or local `Process` `CWL` `Application Package`.
- Add `CLI` output reporting of performed HTTP requests details when using the ``--debug/-d`` option.
- Modify default behavior of ``visibility`` field (under ``processDescription`` or ``processDescription.process``)
to employ the expected functionality by native `OGC API - Processes` clients that do not support this option
(i.e.: ``public`` by default), and to align resolution strategy with deployments by direct `CWL` payload which do not
include this feature either. A `Process` deployment that desires to employ this feature (``visibility: private``) will
have to provide the value explicitly, or update the deployed `Process` definition afterwards with the relevant
``PUT`` request. Since ``public`` will now be used by default, the `CLI` will not automatically inject the value
in the payload anymore when omitted.

Fixes:
------
Expand All @@ -27,6 +34,10 @@ Fixes:
- Fix ``get_cwl_io_type`` function that would modify the I/O definition passed as argument, which could lead to failing
`CWL` ``class`` reference resolutions later on due to different ``type`` with ``org.w3id.cwl.cwl`` prefix simplified
before ``cwltool`` had the chance to resolve them.
- Fix ``links`` listing duplicated in response from `Process` deployment.
Links will only be listed within the returned ``processSummary`` to respect the `OGC API - Processes` schema.
- Fix `CLI` not removing embedded ``links`` in ``processSummary`` from ``deploy`` operation response
when ``-nL``/``--no-links`` option is specified.

.. _changes_4.31.0:

Expand Down
102 changes: 102 additions & 0 deletions tests/functional/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ def tearDownClass(cls):


class TestWeaverClient(TestWeaverClientBase):
test_tmp_dir = None # type: str

@classmethod
def setUpClass(cls):
super(TestWeaverClient, cls).setUpClass()
Expand Down Expand Up @@ -342,6 +344,40 @@ def test_deploy_with_undeploy(self):
assert result.success
assert "undefined" not in result.message

def test_deploy_private_process_description(self):
test_id = f"{self.test_process_prefix}private-process-description"
payload = self.retrieve_payload("Echo", "deploy", local=True)
package = self.retrieve_payload("Echo", "package", local=True)
payload.pop("executionUnit", None)
process = payload["processDescription"].pop("process")
payload["processDescription"].update(process)
payload["processDescription"]["visibility"] = Visibility.PRIVATE

result = mocked_sub_requests(self.app, self.client.deploy, test_id, payload, package)
assert result.success
assert "processSummary" in result.body
assert result.body["processSummary"]["id"] == test_id

result = mocked_sub_requests(self.app, self.client.describe, test_id)
assert not result.success
assert result.code == 403

def test_deploy_private_process_nested(self):
test_id = f"{self.test_process_prefix}private-process-nested"
payload = self.retrieve_payload("Echo", "deploy", local=True)
package = self.retrieve_payload("Echo", "package", local=True)
payload.pop("executionUnit", None)
payload["processDescription"]["process"]["visibility"] = Visibility.PRIVATE

result = mocked_sub_requests(self.app, self.client.deploy, test_id, payload, package)
assert result.success
assert "processSummary" in result.body
assert result.body["processSummary"]["id"] == test_id

result = mocked_sub_requests(self.app, self.client.describe, test_id)
assert not result.success
assert result.code == 403

def test_undeploy(self):
# deploy a new process to leave the test one available
other_payload = copy.deepcopy(self.test_payload["Echo"])
Expand Down Expand Up @@ -771,6 +807,7 @@ def test_deploy_no_process_id_option(self):
"-u", self.url,
"--body", payload, # no --process/--id, but available through --body
"--cwl", package,
"-D", # avoid conflict just in case
],
trim=False,
entrypoint=weaver_cli,
Expand All @@ -779,6 +816,28 @@ def test_deploy_no_process_id_option(self):
assert any("\"id\": \"Echo\"" in line for line in lines)
assert any("\"deploymentDone\": true" in line for line in lines)

def test_deploy_no_links(self):
payload = self.retrieve_payload("Echo", "deploy", local=True, ref_found=True)
package = self.retrieve_payload("Echo", "package", local=True, ref_found=True)
lines = mocked_sub_requests(
self.app, run_command,
[
# "weaver",
"deploy",
"-u", self.url,
"--body", payload,
"--cwl", package,
"-nL",
"-D",
],
trim=False,
entrypoint=weaver_cli,
only_local=True,
)
# ignore indents of fields from formatted JSON content
assert any("\"id\": \"Echo\"" in line for line in lines)
assert all("\"links\":" not in line for line in lines)

def test_deploy_docker_auth_help(self):
"""
Validate some special handling to generate special combinations of help argument details.
Expand Down Expand Up @@ -1255,6 +1314,49 @@ def test_describe_no_links(self):
assert any("\"outputs\": {" in line for line in lines)
assert all("\"links\":" not in line for line in lines)

def test_package_process(self):
payload = self.retrieve_payload("Echo", "deploy", local=True, ref_found=True)
package = self.retrieve_payload("Echo", "package", local=True)
lines = mocked_sub_requests(
self.app, run_command,
[
# weaver
"deploy",
"-u", self.url,
"--body", payload,
"--cwl", package,
"--id", "test-echo-get-package"
],
trim=False,
entrypoint=weaver_cli,
only_local=True,
)
assert any("\"id\": \"test-echo-get-package\"" in line for line in lines)

lines = mocked_sub_requests(
self.app, run_command,
[
# weaver
"package",
"-u", self.url,
"-p", "test-echo-get-package"
],
trim=False,
entrypoint=weaver_cli,
only_local=True,
)
assert any("\"cwlVersion\"" in line for line in lines)
cwl = json.loads("".join(lines))

# package not 100% the same, but equivalent definitions
# check that what is returned is at least relatively equal
cwl.pop("$id", None)
cwl.pop("$schema", None)
pkg = package.copy()
pkg["inputs"] = [dict(id=key, **val) for key, val in package["inputs"].items()]
pkg["outputs"] = [dict(id=key, **val) for key, val in package["outputs"].items()]
assert cwl == pkg

def test_execute_inputs_capture(self):
"""
Verify that specified inputs are captured for a limited number of 1 item per ``-I`` option.
Expand Down
8 changes: 7 additions & 1 deletion tests/functional/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@
from typing import Any, Dict, Iterable, Optional, Tuple, Union
from typing_extensions import Literal

from pyramid.config import Configurator
from webtest import TestApp

from weaver.typedefs import (
AnyRequestMethod,
AnyResponseType,
Expand Down Expand Up @@ -305,7 +308,10 @@ class WpsConfigBase(unittest.TestCase):
json_headers = {"Accept": ContentType.APP_JSON, "Content-Type": ContentType.APP_JSON}
monitor_timeout = 30
monitor_interval = 1
settings = {} # type: SettingsType
settings = {} # type: SettingsType
config = None # type: Configurator
app = None # type: TestApp
url = None # type: str

def __init__(self, *args, **kwargs):
# won't run this as a test suite, only its derived classes
Expand Down
6 changes: 4 additions & 2 deletions weaver/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,8 @@ def _parse_result(response, # type: Union[Response, OperationResult]
for item in nested:
if isinstance(item, dict):
item.pop("links", None)
elif isinstance(nested, dict):
nested.pop("links", None)
body.pop("links", None)
msg = body.get("description", body.get("message", "undefined"))
if code >= 400:
Expand Down Expand Up @@ -484,7 +486,6 @@ def _parse_deploy_body(body, process_id):
desc = data.get("processDescription", {}).get("process", {}) or data.get("processDescription", {})
desc["id"] = process_id
data.setdefault("processDescription", desc) # already applied if description was found/updated at any level
desc["visibility"] = Visibility.PUBLIC
except (ValueError, TypeError, ScannerError) as exc: # pragma: no cover
return OperationResult(False, f"Failed resolution of body definition: [{exc!s}]", body)
return OperationResult(True, "", data)
Expand Down Expand Up @@ -711,7 +712,8 @@ def deploy(self,
resp = self._request("POST", path, json=data,
headers=req_headers, x_headers=headers, settings=self._settings, auth=auth,
request_timeout=request_timeout, request_retries=request_retries)
return self._parse_result(resp, with_links=with_links, with_headers=with_headers, output_format=output_format)
return self._parse_result(resp, with_links=with_links, nested_links="processSummary",
with_headers=with_headers, output_format=output_format)

def undeploy(self,
process_id, # type: str
Expand Down
2 changes: 1 addition & 1 deletion weaver/datatype.py
Original file line number Diff line number Diff line change
Expand Up @@ -2247,7 +2247,7 @@ def estimator(self, estimator):
@property
def visibility(self):
# type: () -> Visibility
return Visibility.get(self.get("visibility"), Visibility.PRIVATE)
return Visibility.get(self.get("visibility"), Visibility.PUBLIC)

@visibility.setter
def visibility(self, visibility):
Expand Down
4 changes: 3 additions & 1 deletion weaver/processes/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -450,6 +450,7 @@ def deploy_process_from_payload(payload, container, overwrite=False): # pylint:
process_info["jobControlOptions"] = process_desc.get("jobControlOptions", [])
process_info["outputTransmission"] = process_desc.get("outputTransmission", [])
process_info["processDescriptionURL"] = description_url

# insert the "resolved" context using details retrieved from "executionUnit"/"href" or directly with "owsContext"
if "owsContext" not in process_info and reference:
process_info["owsContext"] = {"offering": {"content": {"href": str(reference)}}}
Expand All @@ -476,11 +477,12 @@ def deploy_process_from_payload(payload, container, overwrite=False): # pylint:
raise HTTPBadRequest(detail=str(exc))
except HTTPException:
raise
links = process.links(container)
process_summary["links"] = links
data = {
"description": sd.OkPostProcessesResponse.description,
"processSummary": process_summary,
"deploymentDone": True,
"links": process.links(container),
}
if deployment_profile_name:
data["deploymentProfileName"] = deployment_profile_name
Expand Down
8 changes: 0 additions & 8 deletions weaver/wps_restapi/swagger_definitions.py
Original file line number Diff line number Diff line change
Expand Up @@ -4844,14 +4844,6 @@ class Unit(ExtendedMappingSchema):
unit = CWL(description=f"Execution unit definition as CWL package specification. {CWL_DOC_MESSAGE}")


class UndeploymentResult(ExtendedMappingSchema):
id = AnyIdentifier()


class DeploymentResult(ExtendedMappingSchema):
processSummary = ProcessSummary()


class ProviderSummaryList(ExtendedSequenceSchema):
provider_service = ProviderSummarySchema()

Expand Down

0 comments on commit 9f8acab

Please sign in to comment.