diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8209d40..96e7f0e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -62,3 +62,8 @@ repos: - id: interrogate exclude: ^(docs/conf.py|setup.py|tests) args: [--config=pyproject.toml] + +- repo: https://github.com/jendrikseipp/vulture + rev: 'v2.7' + hooks: + - id: vulture diff --git a/Pipfile.lock b/Pipfile.lock index 4342732..9495bc8 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -376,6 +376,7 @@ "hashes": [ "sha256:0f8da300b5c8af9f98111ffd512910bc792b4c77392a9523624680f7956a99d4", "sha256:35f7c7d015d474f4011e859e93e789c87d21f6f4880ebdc29896a60403328f1f", + "sha256:4789d1e3e257965e960232345002262ede4d094d1a19f4d3b52e48d4d8f3b885", "sha256:5aa67414fcdfa22cf052e640cb5ddc461924a045cacf325cd164e65312d99502", "sha256:5d2d8b87a490bfcd407ed9d49093793d0f75198a35e6eb1a923ce1ee86c62b41", "sha256:6687ef6d0a6497e2b58e7c5b852b53f62142cfa7cd1555795758934da363a965", @@ -386,6 +387,7 @@ "sha256:96f1157a7c08b5b189b16b47bc9db2332269d6680a196341bf30046330d15388", "sha256:aec5a6c9864be7df2240c382740fcf3b96928c46604eaa7f3091f58b878c0bb6", "sha256:b0afd054cd42f3d213bf82c629efb1ee5f22eba35bf0eec88ea9ea7304f511a2", + "sha256:c5caeb8188c24888c90b5108a441c106f7faa4c4c075a2bcae438c6e8ca73cef", "sha256:ced4e447ae29ca194449a3f1ce132ded8fcab06971ef5f618605aacaa612beac", "sha256:d1f6198ee6d9148405e49887803907fe8962a23e6c6f83ea7d98f1c0de375695", "sha256:e124352fd3db36a9d4a21c1aa27fd5d051e621845cb87fb851c08f4f75ce8be6", @@ -1336,11 +1338,11 @@ }, "tenacity": { "hashes": [ - "sha256:a43bcd8910406e0884ca0db3db7bed581f389c1d05165e992a1ddabfc81df05e", - "sha256:b723061a78ed0f4524190eae321d3d84100227d51c5677035b6615d91895e0d6" + "sha256:c7bb4b86425b977726a7b49971542d4f67baf72096597d283f3ffd01f33b92df", + "sha256:dd1b769ca7002fda992322939feca5bee4fa11f39146b0af14e0b8d9f27ea854" ], "markers": "python_version >= '3.6'", - "version": "==8.2.0" + "version": "==8.2.1" }, "termcolor": { "hashes": [ @@ -2034,6 +2036,7 @@ "hashes": [ "sha256:0f8da300b5c8af9f98111ffd512910bc792b4c77392a9523624680f7956a99d4", "sha256:35f7c7d015d474f4011e859e93e789c87d21f6f4880ebdc29896a60403328f1f", + "sha256:4789d1e3e257965e960232345002262ede4d094d1a19f4d3b52e48d4d8f3b885", "sha256:5aa67414fcdfa22cf052e640cb5ddc461924a045cacf325cd164e65312d99502", "sha256:5d2d8b87a490bfcd407ed9d49093793d0f75198a35e6eb1a923ce1ee86c62b41", "sha256:6687ef6d0a6497e2b58e7c5b852b53f62142cfa7cd1555795758934da363a965", @@ -2044,6 +2047,7 @@ "sha256:96f1157a7c08b5b189b16b47bc9db2332269d6680a196341bf30046330d15388", "sha256:aec5a6c9864be7df2240c382740fcf3b96928c46604eaa7f3091f58b878c0bb6", "sha256:b0afd054cd42f3d213bf82c629efb1ee5f22eba35bf0eec88ea9ea7304f511a2", + "sha256:c5caeb8188c24888c90b5108a441c106f7faa4c4c075a2bcae438c6e8ca73cef", "sha256:ced4e447ae29ca194449a3f1ce132ded8fcab06971ef5f618605aacaa612beac", "sha256:d1f6198ee6d9148405e49887803907fe8962a23e6c6f83ea7d98f1c0de375695", "sha256:e124352fd3db36a9d4a21c1aa27fd5d051e621845cb87fb851c08f4f75ce8be6", @@ -3744,11 +3748,11 @@ }, "tenacity": { "hashes": [ - "sha256:a43bcd8910406e0884ca0db3db7bed581f389c1d05165e992a1ddabfc81df05e", - "sha256:b723061a78ed0f4524190eae321d3d84100227d51c5677035b6615d91895e0d6" + "sha256:c7bb4b86425b977726a7b49971542d4f67baf72096597d283f3ffd01f33b92df", + "sha256:dd1b769ca7002fda992322939feca5bee4fa11f39146b0af14e0b8d9f27ea854" ], "markers": "python_version >= '3.6'", - "version": "==8.2.0" + "version": "==8.2.1" }, "termcolor": { "hashes": [ @@ -3867,6 +3871,14 @@ "markers": "python_version >= '3.7'", "version": "==20.19.0" }, + "vulture": { + "hashes": [ + "sha256:67fb80a014ed9fdb599dd44bb96cb54311032a104106fc2e706ef7a6dad88032", + "sha256:bccc51064ed76db15a6b58277cea8885936af047f53d2655fb5de575e93d0bca" + ], + "markers": "python_version >= '3.6'", + "version": "==2.7" + }, "wcwidth": { "hashes": [ "sha256:795b138f6875577cd91bba52baf9e445cd5118fd32723b460e30a0af30ea230e", diff --git a/pyproject.toml b/pyproject.toml index 6158b91..831d1c8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,3 +29,13 @@ verbose = true fail-under = 95 ignore-module = true ignore-nested-functions = true + +[tool.vulture] +paths = ["src/orca/"] +# exclude = ["*file*.py", "dir/"] +# ignore_decorators = ["@app.route", "@require_*"] +# ignore_names = ["visit_*", "do_*"] +# make_whitelist = true +min_confidence = 80 +# sort_by_size = true +# verbose = true diff --git a/setup.cfg b/setup.cfg index 972a1bf..f05957b 100644 --- a/setup.cfg +++ b/setup.cfg @@ -90,6 +90,7 @@ dev = sphinx-autodoc-typehints~=1.21 interrogate~=1.5 jupyterlab~=3.6 + vulture~=2.7 [options.entry_points] # Add here console scripts like: @@ -114,7 +115,7 @@ apache_airflow_provider = # Comment those flags to avoid this pytest issue. addopts = --cov "orca" --cov-report "term-missing" --cov-report "xml" - -m "not slow and not integration" + -m "not slow and not integration and not acceptance" --verbose norecursedirs = dist @@ -130,7 +131,7 @@ testpaths = markers = slow: mark tests as slow (deselect with '-m "not slow"') integration: mark tests that interact with external services - # acceptance: mark end-to-end acceptance tests + acceptance: mark end-to-end acceptance tests [devpi:upload] # Options for the devpi: PyPI server and packaging tool diff --git a/src/orca/services/sevenbridges/client_factory.py b/src/orca/services/sevenbridges/client_factory.py index e854a26..21a6912 100644 --- a/src/orca/services/sevenbridges/client_factory.py +++ b/src/orca/services/sevenbridges/client_factory.py @@ -1,15 +1,19 @@ +from __future__ import annotations + import os from dataclasses import field from functools import cached_property -from typing import Any, ClassVar, Optional, Type +from typing import TYPE_CHECKING, Any, ClassVar, Optional, Type -from airflow.models.connection import Connection from pydantic.dataclasses import dataclass from sevenbridges import Api from sevenbridges.http.error_handlers import maintenance_sleeper, rate_limit_sleeper from orca.errors import ClientArgsError, ClientRequestError +if TYPE_CHECKING: + from airflow.models.connection import Connection + @dataclass(kw_only=False) class SevenBridgesClientFactory: @@ -62,14 +66,14 @@ def __post_init_post_parse__(self) -> None: self.update_client_kwargs() @staticmethod - def map_connection(connection: Connection) -> dict[str, Any]: - """Map Airflow connection fields to client arguments. + def parse_connection(connection: Connection) -> dict[str, Optional[str]]: + """Map Airflow connection fields to keyword arguments. Args: connection: An Airflow connection object. Returns: - A dictionary of client arguments. + Keyword arguments relevant to SevenBridges. """ api_endpoint = None if connection.host: @@ -80,9 +84,34 @@ def map_connection(connection: Connection) -> dict[str, Any]: kwargs = { "api_endpoint": api_endpoint, "auth_token": connection.password, + "project": connection.extra_dejson.get("project"), } return kwargs + @classmethod + def connection_from_env(cls) -> Connection: + """Generate Airflow connection from environment variable. + + Returns: + An Airflow connection + """ + # Following Airflow's lead on this non-standard practice + # because this import does introduce a bit of overhead + from airflow.models.connection import Connection + + env_connection_uri = os.environ.get(cls.CONNECTION_ENV) + return Connection(uri=env_connection_uri) + + @classmethod + def kwargs_from_env(cls) -> dict[str, Optional[str]]: + """Parse environment variable for keyword arguments. + + Returns: + Keyword arguments relevant to SevenBridges. + """ + env_connection = cls.connection_from_env() + return cls.parse_connection(env_connection) + def resolve_credentials(self) -> None: """Resolve SevenBridges credentials based on priority. @@ -93,9 +122,7 @@ def resolve_credentials(self) -> None: return # Get value from environment, which is confirmed to be available - env_connection_uri = os.environ[self.CONNECTION_ENV] - env_connection = Connection(uri=env_connection_uri) - env_kwargs = self.map_connection(env_connection) + env_kwargs = self.kwargs_from_env() # Resolve single values for each client argument based on priority self.api_endpoint = self.api_endpoint or env_kwargs["api_endpoint"] diff --git a/src/orca/services/sevenbridges/hook.py b/src/orca/services/sevenbridges/hook.py index 29f7335..c1310c4 100644 --- a/src/orca/services/sevenbridges/hook.py +++ b/src/orca/services/sevenbridges/hook.py @@ -1,11 +1,19 @@ +from __future__ import annotations + from functools import cached_property +from typing import TYPE_CHECKING +from airflow.exceptions import AirflowNotFoundException from airflow.hooks.base import BaseHook from sevenbridges import Api +from orca.errors import ClientArgsError from orca.services.sevenbridges.client_factory import SevenBridgesClientFactory from orca.services.sevenbridges.ops import SevenBridgesOps +if TYPE_CHECKING: + from airflow.models.connection import Connection + class SevenBridgesHook(BaseHook): """Wrapper around SevenBridges client and ops classes. @@ -31,18 +39,45 @@ def __init__(self, conn_id: str = default_conn_name, *args, **kwargs): extras = self.connection.extra_dejson self.project = extras.get("project") - def get_conn(self) -> Api: - """Retrieve authenticated SevenBridges client.""" - return self.client + @classmethod + def get_connection(cls, conn_id: str) -> Connection: + """ + Retrieve Airflow connection + + Args: + conn_id: Airflow connection ID. + + Returns: + An Airflow connection. + """ + try: + connection = super().get_connection(conn_id) + except AirflowNotFoundException: + connection = SevenBridgesClientFactory.connection_from_env() + return connection + + def get_conn(self) -> SevenBridgesOps: + """Retrieve the authenticated SevenBridgesOps object. + + This object contains an authenticated SevenBridges client. + + Returns: + An authenticated SevenBridgesOps instance. + """ + return self.ops @cached_property def client(self) -> Api: """Retrieve authenticated SevenBridges client.""" - client_args = SevenBridgesClientFactory.map_connection(self.connection) - factory = SevenBridgesClientFactory(**client_args) - return factory.get_client() + return self.ops.client @cached_property def ops(self) -> SevenBridgesOps: - """Retrieve authenticated SevenBridgesOps instance.""" - return SevenBridgesOps(self.client, self.project) + """An authenticated SevenBridgesOps instance.""" + kwargs = SevenBridgesClientFactory.parse_connection(self.connection) + endpoint = kwargs.pop("api_endpoint") + token = kwargs.pop("auth_token") + if endpoint is None or token is None: + message = f"Unset 'api_endpoint' ({endpoint}) or 'auth_token' ({token})" + raise ClientArgsError(message) + return SevenBridgesOps.from_args(endpoint, token, **kwargs) diff --git a/src/orca/services/sevenbridges/ops.py b/src/orca/services/sevenbridges/ops.py index a05abbd..442dd1b 100644 --- a/src/orca/services/sevenbridges/ops.py +++ b/src/orca/services/sevenbridges/ops.py @@ -47,14 +47,13 @@ class SevenBridgesOps: project: Optional[str] = None @classmethod - def from_creds( + def from_args( cls, api_endpoint: str, auth_token: str, project: Optional[str] = None, - **client_kwargs: Any, ) -> SevenBridgesOps: - """Construct SevenBridgesOps from credentials. + """Construct SevenBridgesOps from individual arguments. Args: api_endpoint: API base endpoint. @@ -63,13 +62,11 @@ def from_creds( project: An owner-prefixed SevenBridges project. For example: /. Defaults to None. - **client_kwargs: Additional keyword arguments that are passed - to the SevenBridges client during its construction. Returns: An authenticated SevenBridgesOps instance. """ - factory = SevenBridgesClientFactory(api_endpoint, auth_token, client_kwargs) + factory = SevenBridgesClientFactory(api_endpoint, auth_token) client = factory.get_client() return SevenBridgesOps(client, project) diff --git a/tests/acceptance/test_cavatica.py b/tests/acceptance/test_cavatica.py new file mode 100644 index 0000000..bfc2bfc --- /dev/null +++ b/tests/acceptance/test_cavatica.py @@ -0,0 +1,32 @@ +import pytest +from sevenbridges.models.enums import TaskStatus + +from orca.services.sevenbridges import SevenBridgesHook + + +@pytest.mark.acceptance +def test_cavatica_launch_poc_v2(run_id): + def create_task(): + hook = SevenBridgesHook() + task_inputs = { + "input_type": "FASTQ", + "reads1": hook.client.files.get("63e569217a0654635c558c84"), + "reads2": hook.client.files.get("63e5694ebfc712185ac37a27"), + "runThreadN": 36, + "wf_strand_param": "default", + "sample_name": "HCC1187_1M", + "rmats_read_length": 101, + "outSAMattrRGline": "ID:HCC1187_1M\tLB:NA\tPL:Illumina\tSM:HCC1187_1M", + "output_basename": run_id, + } + app_id = "orca-service/test-project/kfdrc-rnaseq-workflow" + task_id = hook.ops.create_task(run_id, app_id, task_inputs) + return task_id + + def monitor_task(task_name): + hook = SevenBridgesHook() + return hook.ops.get_task_status(task_name) + + task_id = create_task() + task_status, _ = monitor_task(task_id) + assert hasattr(TaskStatus, str(task_status)) diff --git a/tests/conftest.py b/tests/conftest.py index a5cb850..e9e9072 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,19 @@ +from datetime import datetime +from getpass import getuser +from uuid import uuid4 + import pytest +UUID = str(uuid4()) +USER = getuser() +UTCTIME = datetime.now().isoformat("T", "seconds").replace(":", ".") +RUN_ID = f"{USER}--{UTCTIME}--{UUID}" # Valid characters: [A-Za-z0-9._-] + + +@pytest.fixture +def run_id(): + return RUN_ID + @pytest.fixture def patch_os_environ(mocker): diff --git a/tests/services/sevenbridges/conftest.py b/tests/services/sevenbridges/conftest.py index b14724c..27c9752 100644 --- a/tests/services/sevenbridges/conftest.py +++ b/tests/services/sevenbridges/conftest.py @@ -96,7 +96,7 @@ def ops_args(client_args): @pytest.fixture def mock_ops(ops_args, mock_api): - yield SevenBridgesOps.from_creds(**ops_args) + yield SevenBridgesOps.from_args(**ops_args) # Note that this refers to a SevenBridges task (or workflow run) diff --git a/tests/services/sevenbridges/test_client_factory.py b/tests/services/sevenbridges/test_client_factory.py index b9d15a9..ff017a8 100644 --- a/tests/services/sevenbridges/test_client_factory.py +++ b/tests/services/sevenbridges/test_client_factory.py @@ -37,16 +37,15 @@ def test_that_the_default_error_handlers_are_used(self, client_args, mock_api_in assert rate_limit_sleeper in handlers -def test_that_a_nonempty_connection_can_be_mapped(connection, client_args): - expected = client_args - actual = SevenBridgesClientFactory.map_connection(connection) - assert actual == expected +def test_that_a_nonempty_connection_can_be_mapped(connection, ops_args): + actual = SevenBridgesClientFactory.parse_connection(connection) + assert actual == ops_args def test_that_an_empty_connection_can_be_mapped(): - expected = {"api_endpoint": None, "auth_token": None} + expected = {"api_endpoint": None, "auth_token": None, "project": None} connection = Connection(uri="sbg://") - result = SevenBridgesClientFactory.map_connection(connection) + result = SevenBridgesClientFactory.parse_connection(connection) assert result == expected diff --git a/tests/services/sevenbridges/test_hook.py b/tests/services/sevenbridges/test_hook.py index 545b6ba..68e5643 100644 --- a/tests/services/sevenbridges/test_hook.py +++ b/tests/services/sevenbridges/test_hook.py @@ -9,8 +9,8 @@ class TestWithoutAirflow: def test_that_a_hook_can_be_constructed_from_a_connection(self, hook): assert isinstance(hook, SevenBridgesHook) - def test_that_get_conn_return_client(self, hook): - assert hook.get_conn() == hook.client + def test_that_get_conn_returns_ops_object(self, hook): + assert hook.get_conn() == hook.ops def test_that_the_client_object_can_be_retrieved_from_hook(self, hook): assert isinstance(hook.client, Api) diff --git a/tests/services/sevenbridges/test_ops.py b/tests/services/sevenbridges/test_ops.py index 8f041ae..e6400a3 100644 --- a/tests/services/sevenbridges/test_ops.py +++ b/tests/services/sevenbridges/test_ops.py @@ -7,7 +7,7 @@ @pytest.mark.usefixtures("patch_os_environ") class TestWithEmptyEnv: def test_that_constructions_from_creds_works(self, client_args, mock_api_init): - SevenBridgesOps.from_creds(**client_args) + SevenBridgesOps.from_args(**client_args) mock_api_init.assert_called_once() def test_for_an_error_when_using_a_project_required_method(self, mock_ops):