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: moves session and url to resource attributes #103

Merged
merged 2 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions .github/workflows/conventional-commits.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ jobs:
fix
perf
style
refactor
test
- uses: marocchino/sticky-pull-request-comment@v2
if: always() && (steps.lint.outputs.error_message != null)
Expand Down
2 changes: 1 addition & 1 deletion src/posit/connect/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def connect_version(self):
def me(self) -> User:
url = urls.append_path(self.config.url, "v1/user")
response = self.session.get(url)
return User(**response.json())
return User(self.session, url, **response.json())

@property
def oauth(self) -> OAuthIntegration:
Expand Down
29 changes: 19 additions & 10 deletions src/posit/connect/content.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from __future__ import annotations

from typing import Any, Callable, List, Optional
from typing import Callable, List, Optional

from requests import Session

Expand Down Expand Up @@ -183,9 +183,6 @@ def app_role(self) -> str:
def id(self) -> str:
return self.get("id") # type: ignore

def __setattr__(self, name: str, value: Any) -> None:
raise AttributeError("Cannot set attributes: use update() instead")


class Content(Resources[ContentItem]):
def __init__(self, config: Config, session: Session) -> None:
Expand All @@ -197,22 +194,34 @@ def find(
self, filter: Callable[[ContentItem], bool] = lambda _: True
) -> List[ContentItem]:
results = self.session.get(self.url).json()
return [ContentItem(**c) for c in results if filter(ContentItem(**c))]
items = (
ContentItem(
session=self.session,
url=urls.append_path(self.url, result["guid"]),
**result,
)
for result in results
)
return [item for item in items if filter(item)]

def find_one(
self, filter: Callable[[ContentItem], bool] = lambda _: True
) -> ContentItem | None:
results = self.session.get(self.url).json()
for c in results:
content_item = ContentItem(**c)
if filter(content_item):
return content_item
for result in results:
item = ContentItem(
session=self.session,
url=urls.append_path(self.url, result["guid"]),
**result,
)
if filter(item):
return item
return None

def get(self, id: str) -> ContentItem:
url = urls.append_path(self.url, id)
response = self.session.get(url)
return ContentItem(**response.json())
return ContentItem(self.session, url, **response.json())

def create(self) -> ContentItem:
raise NotImplementedError()
Expand Down
14 changes: 13 additions & 1 deletion src/posit/connect/resources.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,22 @@
import warnings

from abc import ABC, abstractmethod
from typing import Generic, List, Optional, TypeVar
from typing import Any, Generic, List, Optional, TypeVar

import requests


T = TypeVar("T")


class Resource(ABC, dict):
def __init__(self, session: requests.Session, url: str, **kwargs):
tdstein marked this conversation as resolved.
Show resolved Hide resolved
super().__init__(**kwargs)
self.session: requests.Session
super().__setattr__("session", session)
self.url: str
super().__setattr__("url", url)

def __getitem__(self, key):
warnings.warn(
f"__getitem__ for '{key}' does not support backwards compatibility. Consider using field based access instead: 'instance.{key}'",
Expand All @@ -29,6 +38,9 @@ def __delitem__(self, key):
)
return super().__delitem__(key)

def __setattr__(self, name: str, value: Any) -> None:
raise AttributeError("cannot set attributes: use update() instead")


class Resources(ABC, Generic[T]):
@abstractmethod
Expand Down
19 changes: 8 additions & 11 deletions src/posit/connect/users.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from __future__ import annotations
from typing import Any, Callable, List, Optional
from typing import Callable, List, Optional


from requests import Session
Expand Down Expand Up @@ -57,11 +57,8 @@ def confirmed(self) -> bool:
def locked(self) -> bool:
return self.get("locked") # type: ignore

def __setattr__(self, name: str, value: Any) -> None:
raise AttributeError("Cannot set attributes: use update() instead.")

def _update(self, body):
self.get("session").patch(self.get("url"), json=body)
self.session.patch(self.url, json=body)
# If the request is successful, update the local object
super().update(body)
# TODO(#99): that patch request returns a payload on success,
Expand Down Expand Up @@ -107,15 +104,15 @@ def find(
self, filter: Callable[[User], bool] = lambda _: True, page_size=_MAX_PAGE_SIZE
) -> List[User]:
results = Paginator(self.session, self.url, page_size=page_size).get_all()
return [
users = (
User(
**user,
session=self.session,
url=urls.append_path(self.url, user["guid"]),
**user,
)
for user in results
if filter(User(**user))
]
)
return [user for user in users if filter(user)]

def find_one(
self, filter: Callable[[User], bool] = lambda _: True, page_size=_MAX_PAGE_SIZE
Expand All @@ -125,9 +122,9 @@ def find_one(
result = pager.get_next_page()
for u in result:
user = User(
**u,
session=self.session,
url=urls.append_path(self.url, u["guid"]),
**u,
)
if filter(user):
return user
Expand All @@ -138,9 +135,9 @@ def get(self, id: str) -> User:
response = self.session.get(url)
raw_user = response.json()
return User(
**raw_user,
session=self.session,
url=urls.append_path(self.url, raw_user["guid"]),
**raw_user,
)

def create(self) -> User:
Expand Down
24 changes: 20 additions & 4 deletions tests/posit/connect/test_resources.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
from unittest.mock import Mock
import pytest
import warnings

from typing import Any, List, Optional

from requests.sessions import Session as Session

from posit.connect.resources import Resource, Resources

session = Mock()
url = Mock()


class FakeResource(Resource):
@property
Expand All @@ -13,12 +19,20 @@ def foo(self) -> Optional[str]:


class TestResource:
def test_init(self):
k = "foo"
v = "bar"
d = dict({k: v})
r = FakeResource(session, url, **d)
assert r.session == session
assert r.url == url

def test__getitem__(self):
warnings.filterwarnings("ignore", category=FutureWarning)
k = "foo"
v = "bar"
d = dict({k: v})
r = FakeResource(d)
r = FakeResource(session, url, **d)
assert r.__getitem__(k) == d.__getitem__(k)
assert r[k] == d[k]

Expand All @@ -27,7 +41,8 @@ def test__setitem__(self):
k = "foo"
v1 = "bar"
v2 = "baz"
r = FakeResource({k: v1})
d = dict({k: v1})
r = FakeResource(session, url, **d)
assert r[k] == v1
r[k] = v2
assert r[k] == v2
Expand All @@ -36,7 +51,8 @@ def test__delitem__(self):
warnings.filterwarnings("ignore", category=FutureWarning)
k = "foo"
v = "bar"
r = FakeResource({k: v})
d = dict({k: v})
r = FakeResource(session, url, **d)
assert k in r
assert r[k] == v
del r[k]
Expand All @@ -46,7 +62,7 @@ def test_foo(self):
k = "foo"
v = "bar"
d = dict({k: v})
r = FakeResource(d)
r = FakeResource(session, url, **d)
assert r.foo == v


Expand Down
54 changes: 28 additions & 26 deletions tests/posit/connect/test_users.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from unittest.mock import Mock
import pandas as pd
import pytest
import responses
Expand All @@ -9,83 +10,86 @@

from .api import load_mock # type: ignore

session = Mock()
url = Mock()


class TestUser:
def test_guid(self):
user = User()
user = User(session, url)
assert hasattr(user, "guid")
assert user.guid is None
user = User({"guid": "test_guid"})
user = User(session, url, guid="test_guid")
assert user.guid == "test_guid"

def test_email(self):
user = User()
user = User(session, url)
assert hasattr(user, "email")
assert user.email is None
user = User({"email": "[email protected]"})
user = User(session, url, email="[email protected]")
assert user.email == "[email protected]"

def test_username(self):
user = User()
user = User(session, url)
assert hasattr(user, "username")
assert user.username is None
user = User({"username": "test_user"})
user = User(session, url, username="test_user")
assert user.username == "test_user"

def test_first_name(self):
user = User()
user = User(session, url)
assert hasattr(user, "first_name")
assert user.first_name is None
user = User({"first_name": "John"})
user = User(session, url, first_name="John")
assert user.first_name == "John"

def test_last_name(self):
user = User()
user = User(session, url)
assert hasattr(user, "last_name")
assert user.last_name is None
user = User({"last_name": "Doe"})
user = User(session, url, last_name="Doe")
assert user.last_name == "Doe"

def test_user_role(self):
user = User()
user = User(session, url)
assert hasattr(user, "user_role")
assert user.user_role is None
user = User({"user_role": "admin"})
user = User(session, url, user_role="admin")
assert user.user_role == "admin"

def test_created_time(self):
user = User()
user = User(session, url)
assert hasattr(user, "created_time")
assert user.created_time is None
user = User({"created_time": "2022-01-01T00:00:00"})
user = User(session, url, created_time="2022-01-01T00:00:00")
assert user.created_time == "2022-01-01T00:00:00"

def test_updated_time(self):
user = User()
user = User(session, url)
assert hasattr(user, "updated_time")
assert user.updated_time is None
user = User({"updated_time": "2022-01-01T00:00:00"})
user = User(session, url, updated_time="2022-01-01T00:00:00")
assert user.updated_time == "2022-01-01T00:00:00"

def test_active_time(self):
user = User()
user = User(session, url)
assert hasattr(user, "active_time")
assert user.active_time is None
user = User({"active_time": "2022-01-01T00:00:00"})
user = User(session, url, active_time="2022-01-01T00:00:00")
assert user.active_time == "2022-01-01T00:00:00"

def test_confirmed(self):
user = User()
user = User(session, url)
assert hasattr(user, "confirmed")
assert user.confirmed is None
user = User({"confirmed": True})
user = User(session, url, confirmed=True)
assert user.confirmed is True

def test_locked(self):
user = User()
user = User(session, url)
assert hasattr(user, "locked")
assert user.locked is None
user = User({"locked": False})
user = User(session, url, locked=False)
assert user.locked is False


Expand Down Expand Up @@ -117,7 +121,7 @@ def test_users_find(self):

df = pd.DataFrame(all_users)
assert isinstance(df, pd.DataFrame)
assert df.shape == (3, 13)
assert df.shape == (3, 11)
assert df.columns.to_list() == [
"email",
"username",
Expand All @@ -130,8 +134,6 @@ def test_users_find(self):
"confirmed",
"locked",
"guid",
"session",
"url",
]
assert df["username"].to_list() == ["al", "robert", "carlos12"]

Expand Down Expand Up @@ -263,6 +265,6 @@ def test_user_cant_setattr(self):

with pytest.raises(
AttributeError,
match=r"Cannot set attributes: use update\(\) instead",
match=r"cannot set attributes: use update\(\) instead",
):
carlos.first_name = "Carlitos"
12 changes: 4 additions & 8 deletions tinkering.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
from posit.connect import Client

with Client() as client:
print(client.get("v1/users"))
print(client.users.get("f55ca95d-ce52-43ed-b31b-48dc4a07fe13"))

users = client.users
users = users.find(lambda user: user["first_name"].startswith("T"))
users = users.find(lambda user: user["last_name"].startswith("S"))
user = users.find_one(lambda user: user["user_role"] == "administrator")
print(user)
client.get("v1/users")
client.users.get("f55ca95d-ce52-43ed-b31b-48dc4a07fe13")
client.users.find(lambda user: user.first_name.startswith("T"))
client.users.find_one(lambda user: user.first_name.startswith("T"))
Loading