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

refactor: reduces config complexity #13

Merged
merged 3 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
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
59 changes: 42 additions & 17 deletions src/posit/connect/client.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,50 @@
import os

from requests import Session
from typing import Optional

from . import hooks

from .auth import Auth
from .config import ConfigBuilder
from .users import Users


def _get_api_key() -> str:
"""Gets the API key from the environment variable 'CONNECT_API_KEY'.

Raises:
ValueError: if CONNECT_API_KEY is not set or invalid

Returns:
The API key
"""
value = os.environ.get("CONNECT_API_KEY")
if value is None or value == "":
raise ValueError(
"Invalid value for 'CONNECT_API_KEY': Must be a non-empty string."
)
return value


def _get_endpoint() -> str:
"""Gets the endpoint from the environment variable 'CONNECT_SERVER'.

The `requests` library uses 'endpoint' instead of 'server'. We will use 'endpoint' from here forward for consistency.

Raises:
ValueError: if CONNECT_SERVER is not set or invalid.

Returns:
The endpoint.
"""
value = os.environ.get("CONNECT_SERVER")
if value is None or value == "":
raise ValueError(
"Invalid value for 'CONNECT_SERVER': Must be a non-empty string."
)
return value


class Client:
users: Users

Expand All @@ -16,21 +53,9 @@ def __init__(
api_key: Optional[str] = None,
endpoint: Optional[str] = None,
) -> None:
builder = ConfigBuilder()
if api_key:
builder.set_api_key(api_key)
if endpoint:
builder.set_endpoint(endpoint)
self._config = builder.build()

if self._config.api_key is None:
raise ValueError("Invalid value for 'api_key': Must be a non-empty string.")
if self._config.endpoint is None:
raise ValueError(
"Invalid value for 'endpoint': Must be a non-empty string."
)

self._api_key = api_key or _get_api_key()
self._endpoint = endpoint if endpoint else _get_endpoint()
tdstein marked this conversation as resolved.
Show resolved Hide resolved
self._session = Session()
self._session.hooks["response"].append(hooks.handle_errors)
self._session.auth = Auth(self._config.api_key)
self.users = Users(self._config.endpoint, self._session)
self._session.auth = Auth(self._api_key)
self.users = Users(self._endpoint, self._session)
74 changes: 35 additions & 39 deletions src/posit/connect/client_test.py
Original file line number Diff line number Diff line change
@@ -1,56 +1,52 @@
import pytest

from unittest.mock import MagicMock, Mock, patch
from unittest.mock import MagicMock, patch

from .client import Client
from .client import Client, _get_api_key, _get_endpoint


class TestClient:
@patch("posit.connect.client.Users")
@patch("posit.connect.client.Session")
@patch("posit.connect.client.ConfigBuilder")
@patch("posit.connect.client.Auth")
def test_init(self, Auth: MagicMock, ConfigBuilder: MagicMock, Session: MagicMock):
def test_init(self, Auth: MagicMock, Session: MagicMock, Users: MagicMock):
api_key = "foobar"
endpoint = "http://foo.bar"
config = Mock()
config.api_key = api_key
builder = ConfigBuilder.return_value
builder.set_api_key = Mock()
builder.set_endpoint = Mock()
builder.build = Mock(return_value=config)
client = Client(api_key=api_key, endpoint=endpoint)
ConfigBuilder.assert_called_once()
builder.set_api_key.assert_called_once_with(api_key)
builder.set_endpoint.assert_called_once_with(endpoint)
builder.build.assert_called_once()
assert client._api_key == api_key
assert client._endpoint == endpoint
Session.assert_called_once()
Auth.assert_called_once_with(api_key)
assert client._config == config
Users.assert_called_once_with(endpoint, Session.return_value)

@patch("posit.connect.client.ConfigBuilder")
def test_init_without_api_key(self, ConfigBuilder: MagicMock):
api_key = None
endpoint = "http://foo.bar"
config = Mock()
config.api_key = api_key
config.endpoint = endpoint
builder = ConfigBuilder.return_value
builder.set_api_key = Mock()
builder.set_endpoint = Mock()
builder.build = Mock(return_value=config)

class TestGetApiKey:
@patch.dict("os.environ", {"CONNECT_API_KEY": "foobar"})
def test_get_api_key(self):
api_key = _get_api_key()
assert api_key == "foobar"

@patch.dict("os.environ", {"CONNECT_API_KEY": ""})
def test_get_api_key_empty(self):
with pytest.raises(ValueError):
Client(api_key=api_key, endpoint=endpoint)
_get_api_key()

@patch("posit.connect.client.ConfigBuilder")
def test_init_without_endpoint(self, ConfigBuilder: MagicMock):
api_key = "foobar"
endpoint = None
config = Mock()
config.api_key = api_key
config.endpoint = endpoint
builder = ConfigBuilder.return_value
builder.set_api_key = Mock()
builder.set_endpoint = Mock()
builder.build = Mock(return_value=config)
def test_get_api_key_miss(self):
with pytest.raises(ValueError):
_get_api_key()


class TestGetEndpoint:
@patch.dict("os.environ", {"CONNECT_SERVER": "http://foo.bar"})
def test_get_endpoint(self):
endpoint = _get_endpoint()
assert endpoint == "http://foo.bar"

@patch.dict("os.environ", {"CONNECT_SERVER": ""})
def test_get_endpoint_empty(self):
with pytest.raises(ValueError):
_get_endpoint()

def test_get_endpoint_miss(self):
with pytest.raises(ValueError):
Client(api_key=api_key, endpoint=endpoint)
_get_endpoint()
68 changes: 0 additions & 68 deletions src/posit/connect/config.py

This file was deleted.

70 changes: 0 additions & 70 deletions src/posit/connect/config_test.py

This file was deleted.

4 changes: 2 additions & 2 deletions src/posit/connect/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ def __init__(self, endpoint: str, session: Session) -> None:
self._session = session

def get_user(self, user_id: str) -> Response:
endpoint = os.path.join(self._endpoint, "v1/users", user_id)
endpoint = os.path.join(self._endpoint, "__api__/v1/users", user_id)
return self._session.get(endpoint)

def get_current_user(self) -> Response:
endpoint = os.path.join(self._endpoint, "v1/user")
endpoint = os.path.join(self._endpoint, "__api__/v1/user")
return self._session.get(endpoint)
4 changes: 2 additions & 2 deletions src/posit/connect/users_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ def test_get_user(self):
users = Users(endpoint="http://foo.bar/", session=session)
response = users.get_user(user_id="foo")
assert response == {}
session.get.assert_called_once_with("http://foo.bar/v1/users/foo")
session.get.assert_called_once_with("http://foo.bar/__api__/v1/users/foo")

def test_get_current_user(self):
session = Mock()
session.get = Mock(return_value={})
users = Users(endpoint="http://foo.bar/", session=session)
response = users.get_current_user()
assert response == {}
session.get.assert_called_once_with("http://foo.bar/v1/user")
session.get.assert_called_once_with("http://foo.bar/__api__/v1/user")
Loading