Skip to content

Commit

Permalink
Address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Bruno Grande committed Feb 8, 2023
1 parent 36db3d0 commit 5202894
Show file tree
Hide file tree
Showing 9 changed files with 124 additions and 20 deletions.
1 change: 1 addition & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ repos:
rev: 'v0.991'
hooks:
- id: mypy
additional_dependencies: [pydantic~=1.10]

- repo: https://github.com/kynan/nbstripout
rev: '0.6.1'
Expand Down
10 changes: 5 additions & 5 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,15 @@ This often provides additional considerations and avoids unnecessary work.
cd orca
```

4. You should run:
4. Install `pipx` to easily run Python CLI tools like `tox` and `pipenv`.

4. Create an isolated virtual environment containing package dependencies,
including those needed for development (*e.g.* testing, documentation) by running:

```console
pipenv install --dev
pipx run tox -e pipenv
```

to create an isolated virtual environment containing package dependencies,
including those needed for development (*e.g.* testing, documentation).

5. Install [pre-commit] hooks:

```
Expand Down
84 changes: 84 additions & 0 deletions Pipfile.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions docs/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@

apache-airflow~=2.4
myst-parser[linkify]
pydantic~=1.10
sevenbridges-python~=2.9
sphinx>=3.2.1
sphinx-autodoc-typehints~=1.21
sphinx-rtd-theme~=1.0
sqlalchemy<2.0
8 changes: 7 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,13 @@ build-backend = "setuptools.build_meta"
version_scheme = "no-guess-dev"

[tool.mypy]
disable_error_code = "type-abstract"
plugins = [
"pydantic.mypy"
]
disable_error_code = [
"type-abstract",
"import"
]

[[tool.mypy.overrides]]
module = "orca"
Expand Down
4 changes: 3 additions & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,11 @@ python_requires = >=3.8
# For more information, check out https://semver.org/.

# Updates here should be reflected in `docs/requirements.txt`
# Also, run `tox -e pipenv` to update your virtual environment
install_requires =
apache-airflow~=2.4
sqlalchemy<2.0
pydantic~=1.10
sqlalchemy<2.0 # To address SQLAlchemy warning (https://sqlalche.me/e/b8d9)

[options.packages.find]
where = src
Expand Down
8 changes: 7 additions & 1 deletion src/orca/services/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,11 @@
class from `tasks.py` using the credentials defined by an Airflow
connection. These hook classes are deliberately thin wrappers around
the tasks classes. The aim here is to minimize Airflow-specific
code, which is harder to test.
code, which is harder to test. This hook also takes advantage of
extra field associated with connection objects to configure the
underlying client and/or tasks objects. Whether to include a value
as the extra connection field probably depends on whether that
value is useful to change the overall state of the underlying
client and/or tasks objects (e.g., `project` for SevenBridgesTasks)
and whether the value is invariable for a given DAG.
"""
22 changes: 12 additions & 10 deletions src/orca/services/sevenbridges/client_factory.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
import os
from dataclasses import dataclass, field
from dataclasses import field
from functools import cached_property
from typing import Any, Optional

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


@dataclass
@dataclass(kw_only=False)
class SevenBridgesClientFactory:
"""Factory for constructing SevenBridges clients.
Expand Down Expand Up @@ -49,7 +50,8 @@ class SevenBridgesClientFactory:
"https://cavatica-api.sbgenomics.com/v2",
}

def __post_init__(self):
# Using `__post_init_post_parse__()` to perform steps after validation
def __post_init_post_parse__(self) -> None:
"""Resolve and validate credentials."""
self.resolve_credentials()
self.update_credentials()
Expand Down Expand Up @@ -78,7 +80,7 @@ def map_connection(connection: Connection) -> dict[str, Any]:
}
return kwargs

def resolve_credentials(self):
def resolve_credentials(self) -> None:
"""Resolve SevenBridges credentials based on priority.
This method will update the attribute values (if applicable).
Expand All @@ -87,21 +89,21 @@ def resolve_credentials(self):
if os.environ.get(self.CONNECTION_ENV) is None:
return

# Retrieve information from environment (if available)
env_connection_uri = os.environ.get(self.CONNECTION_ENV, "")
# 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)

# Resolve single values for each client argument based on priority
self.api_endpoint = self.api_endpoint or env_kwargs["api_endpoint"]
self.auth_token = self.auth_token or env_kwargs["auth_token"]

def update_credentials(self):
def update_credentials(self) -> None:
"""Update credentials for consistency."""
if isinstance(self.api_endpoint, str):
self.api_endpoint = self.api_endpoint.rstrip("/")

def validate_credentials(self):
def validate_credentials(self) -> None:
"""Validate the currently available credential attributes.
Raises:
Expand Down Expand Up @@ -132,7 +134,7 @@ def validate_credentials(self):
addendum = f"Auth auth_token ({self.auth_token}) should be a string."
raise ClientArgsError(common_message + addendum)

def update_client_kwargs(self):
def update_client_kwargs(self) -> None:
"""Update client keyword arguments with default values."""
default_handlers = [rate_limit_sleeper, maintenance_sleeper]
self.client_kwargs.setdefault("error_handlers", default_handlers)
Expand Down Expand Up @@ -161,7 +163,7 @@ def get_and_test_client(self) -> Api:
return client

@staticmethod
def test_client(client: Api):
def test_client(client: Api) -> None:
"""Test the client with an authenticated request.
Raises:
Expand Down
5 changes: 3 additions & 2 deletions src/orca/services/sevenbridges/tasks.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
from __future__ import annotations

from dataclasses import dataclass
from typing import Any, Optional

from pydantic import ConfigDict
from pydantic.dataclasses import dataclass
from sevenbridges import Api

from orca.services.sevenbridges.client_factory import SevenBridgesClientFactory


@dataclass
@dataclass(config=ConfigDict(arbitrary_types_allowed=True), kw_only=False)
class SevenBridgesTasks:
"""Common tasks for SevenBridges platforms.
Expand Down

0 comments on commit 5202894

Please sign in to comment.