Skip to content

Commit

Permalink
fix(identity): remove class vars from Identity
Browse files Browse the repository at this point in the history
The default Identity class had a couple class variables (name, id, email) that seemed more like a protocol kind of thing (stating an _instance_ o the class has those attributes), but being it an ABC that's inherited from made the inherited classes never lose those dud class vars, although the instances got their own (as seen in DefaultIdentity).

This might have so far been a little clutter and nothing else, but sice the `GithubIdentity(Identity)` uses an auxiliary dataclass to hold part of these variables, I had to resort to a rather complicated `__getattribute__` override to make access of these aux attributes transparent (because a much simpler `__getattr__` override wasn't possible due to it accessing the dud class vars first).

Providing a simple `__init__` implementation for the base `Identity` works perfectly fine with static checkers and lets me avoid the recursive hell of `__getattribute__` override.
  • Loading branch information
vit-zikmund committed May 13, 2024
1 parent 6a1cb82 commit ad82490
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 14 deletions.
13 changes: 5 additions & 8 deletions giftless/auth/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,10 @@ def __init__(
token_data: Mapping[str, Any],
cc: CacheConfig,
) -> None:
super().__init__()
super().__init__(
token_data.get("name"), core_identity.id, token_data.get("email")
)
self.core_identity = core_identity
self.name = token_data.get("name")
self.email = token_data.get("email")

# Expiring cache of authorized repos with different TTL for each
# permission type. It's assumed that anyone granted the WRITE
Expand All @@ -293,12 +293,9 @@ def expiration(_key: Any, value: set[Permission], now: float) -> float:
self._auth_cache = cachetools.TLRUCache(cc.auth_max_size, expiration)
self._auth_cache_lock = Lock()

def __getattribute__(self, attr: str) -> Any:
def __getattr__(self, attr: str) -> Any:
# proxy to the core_identity for its attributes
core_identity = super().__getattribute__("core_identity")
if attr in core_identity.__slots__:
return getattr(core_identity, attr)
return super().__getattribute__(attr)
return getattr(self.core_identity, attr)

def permissions(
self, org: str, repo: str, *, authoritative: bool = False
Expand Down
16 changes: 10 additions & 6 deletions giftless/auth/identity.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,15 @@ class Identity(ABC):
perform some actions.
"""

name: str | None = None
id: str | None = None
email: str | None = None
def __init__(
self,
name: str | None = None,
id: str | None = None,
email: str | None = None,
) -> None:
self.name = name
self.id = id
self.email = email

@abstractmethod
def is_authorized(
Expand All @@ -58,9 +64,7 @@ def __init__(
id: str | None = None,
email: str | None = None,
) -> None:
self.name = name
self.id = id
self.email = email
super().__init__(name, id, email)
self._allowed: PermissionTree = defaultdict(
lambda: defaultdict(lambda: defaultdict(set))
)
Expand Down

0 comments on commit ad82490

Please sign in to comment.