Skip to content

Commit

Permalink
fix: use correct args and kwargs definitions (#256)
Browse files Browse the repository at this point in the history
Various methods are defined with improper vardadic positional and keyword-arguments. The original intention for @overload signatures was to provide type support for passthroughs to API query parameter or json arguments. Along the way, *args was introduced to these methods assuming that we wanted to match the dict function signature. In practice, this is confusing since passing SupportsKeysAndGetItem typed variables is rare. Threfore, the *args part of the function signature is removed wherever possible. In the case of update(self, *args, **kwargs) we cannot remove *args since we inherit from dict and dict enforces this method signature.
  • Loading branch information
tdstein authored Aug 9, 2024
1 parent 3677c0b commit 84371b0
Show file tree
Hide file tree
Showing 9 changed files with 70 additions and 67 deletions.
10 changes: 6 additions & 4 deletions src/posit/connect/content.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ def restart(self) -> None:
@overload
def update(
self,
*args,
name: str = ...,
title: Optional[str] = ...,
description: str = ...,
Expand All @@ -254,6 +255,7 @@ def update(
default_r_environment_management: Optional[bool] = ...,
default_py_environment_management: Optional[bool] = ...,
service_account_name: Optional[str] = ...,
**kwargs,
) -> None:
"""Update the content item.
Expand Down Expand Up @@ -578,6 +580,7 @@ def count(self) -> int:
@overload
def create(
self,
*,
name: str = ...,
title: Optional[str] = ...,
description: str = ...,
Expand Down Expand Up @@ -639,7 +642,7 @@ def create(
...

@overload
def create(self, *args, **kwargs) -> ContentItem:
def create(self, **kwargs) -> ContentItem:
"""Create a content item.
Returns
Expand All @@ -648,17 +651,16 @@ def create(self, *args, **kwargs) -> ContentItem:
"""
...

def create(self, *args, **kwargs) -> ContentItem:
def create(self, **kwargs) -> ContentItem:
"""Create a content item.
Returns
-------
ContentItem
"""
body = dict(*args, **kwargs)
path = "v1/content"
url = self.url + path
response = self.session.post(url, json=body)
response = self.session.post(url, json=kwargs)
return ContentItem(self.params, **response.json())

@overload
Expand Down
26 changes: 13 additions & 13 deletions src/posit/connect/groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class Groups(Resources):
"""Groups resource."""

@overload
def create(self, name: str, unique_id: str | None) -> Group:
def create(self, *, name: str, unique_id: str | None) -> Group:
"""Create a group.
Parameters
Expand All @@ -60,7 +60,7 @@ def create(self, name: str, unique_id: str | None) -> Group:
...

@overload
def create(self, *args, **kwargs) -> Group:
def create(self, **kwargs) -> Group:
"""Create a group.
Returns
Expand All @@ -69,7 +69,7 @@ def create(self, *args, **kwargs) -> Group:
"""
...

def create(self, *args, **kwargs) -> Group:
def create(self, **kwargs) -> Group:
"""Create a group.
Parameters
Expand All @@ -82,22 +82,22 @@ def create(self, *args, **kwargs) -> Group:
Group
"""
...
body = dict(*args, **kwargs)
path = "v1/groups"
url = self.url + path
response = self.session.post(url, json=body)
response = self.session.post(url, json=kwargs)
return Group(self.params, **response.json())

@overload
def find(
self,
*,
prefix: str = ...,
) -> List[Group]: ...

@overload
def find(self, *args, **kwargs) -> List[Group]: ...
def find(self, **kwargs) -> List[Group]: ...

def find(self, *args, **kwargs):
def find(self, **kwargs):
"""Find groups.
Parameters
Expand All @@ -109,10 +109,9 @@ def find(self, *args, **kwargs):
-------
List[Group]
"""
params = dict(*args, **kwargs)
path = "v1/groups"
url = self.url + path
paginator = Paginator(self.session, url, params=params)
paginator = Paginator(self.session, url, params=kwargs)
results = paginator.fetch_results()
return [
Group(
Expand All @@ -125,13 +124,14 @@ def find(self, *args, **kwargs):
@overload
def find_one(
self,
*,
prefix: str = ...,
) -> Group | None: ...

@overload
def find_one(self, *args, **kwargs) -> Group | None: ...
def find_one(self, **kwargs) -> Group | None: ...

def find_one(self, *args, **kwargs) -> Group | None:
def find_one(self, **kwargs) -> Group | None:
"""Find one group.
Parameters
Expand All @@ -143,10 +143,10 @@ def find_one(self, *args, **kwargs) -> Group | None:
-------
Group | None
"""
params = dict(*args, **kwargs)
dict
path = "v1/groups"
url = self.url + path
paginator = Paginator(self.session, url, params=params)
paginator = Paginator(self.session, url, params=kwargs)
pages = paginator.fetch_pages()
results = (result for page in pages for result in page.results)
groups = (
Expand Down
16 changes: 8 additions & 8 deletions src/posit/connect/metrics/shiny_usage.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ class ShinyUsage(Resources):
@overload
def find(
self,
*,
content_guid: str = ...,
min_data_version: int = ...,
start: str = ...,
Expand All @@ -87,7 +88,7 @@ def find(
...

@overload
def find(self, *args, **kwargs) -> List[ShinyUsageEvent]:
def find(self, **kwargs) -> List[ShinyUsageEvent]:
"""Find usage.
Returns
Expand All @@ -96,15 +97,14 @@ def find(self, *args, **kwargs) -> List[ShinyUsageEvent]:
"""
...

def find(self, *args, **kwargs) -> List[ShinyUsageEvent]:
def find(self, **kwargs) -> List[ShinyUsageEvent]:
"""Find usage.
Returns
-------
List[ShinyUsageEvent]
"""
params = dict(*args, **kwargs)
params = rename_params(params)
params = rename_params(kwargs)

path = "/v1/instrumentation/shiny/usage"
url = self.url + path
Expand All @@ -121,6 +121,7 @@ def find(self, *args, **kwargs) -> List[ShinyUsageEvent]:
@overload
def find_one(
self,
*,
content_guid: str = ...,
min_data_version: int = ...,
start: str = ...,
Expand All @@ -146,7 +147,7 @@ def find_one(
...

@overload
def find_one(self, *args, **kwargs) -> ShinyUsageEvent | None:
def find_one(self, **kwargs) -> ShinyUsageEvent | None:
"""Find a usage event.
Returns
Expand All @@ -155,15 +156,14 @@ def find_one(self, *args, **kwargs) -> ShinyUsageEvent | None:
"""
...

def find_one(self, *args, **kwargs) -> ShinyUsageEvent | None:
def find_one(self, **kwargs) -> ShinyUsageEvent | None:
"""Find a usage event.
Returns
-------
ShinyUsageEvent | None
"""
params = dict(*args, **kwargs)
params = rename_params(params)
params = rename_params(kwargs)
path = "/v1/instrumentation/shiny/usage"
url = self.url + path
paginator = CursorPaginator(self.session, url, params=params)
Expand Down
14 changes: 8 additions & 6 deletions src/posit/connect/metrics/usage.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ class Usage(resources.Resources):
@overload
def find(
self,
*,
content_guid: str = ...,
min_data_version: int = ...,
start: str = ...,
Expand All @@ -179,7 +180,7 @@ def find(
...

@overload
def find(self, *args, **kwargs) -> List[UsageEvent]:
def find(self, **kwargs) -> List[UsageEvent]:
"""Find view events.
Returns
Expand All @@ -188,7 +189,7 @@ def find(self, *args, **kwargs) -> List[UsageEvent]:
"""
...

def find(self, *args, **kwargs) -> List[UsageEvent]:
def find(self, **kwargs) -> List[UsageEvent]:
"""Find view events.
Returns
Expand All @@ -202,14 +203,15 @@ def find(self, *args, **kwargs) -> List[UsageEvent]:
events.extend(
[
UsageEvent.from_event(event)
for event in instance.find(*args, **kwargs) # type: ignore[attr-defined]
for event in instance.find(**kwargs) # type: ignore[attr-defined]
]
)
return events

@overload
def find_one(
self,
*,
content_guid: str = ...,
min_data_version: int = ...,
start: str = ...,
Expand All @@ -235,7 +237,7 @@ def find_one(
...

@overload
def find_one(self, *args, **kwargs) -> UsageEvent | None:
def find_one(self, **kwargs) -> UsageEvent | None:
"""Find a view event.
Returns
Expand All @@ -244,7 +246,7 @@ def find_one(self, *args, **kwargs) -> UsageEvent | None:
"""
...

def find_one(self, *args, **kwargs) -> UsageEvent | None:
def find_one(self, **kwargs) -> UsageEvent | None:
"""Find a view event.
Returns
Expand All @@ -254,7 +256,7 @@ def find_one(self, *args, **kwargs) -> UsageEvent | None:
finders = (visits.Visits, shiny_usage.ShinyUsage)
for finder in finders:
instance = finder(self.params)
event = instance.find_one(*args, **kwargs) # type: ignore[attr-defined]
event = instance.find_one(**kwargs) # type: ignore[attr-defined]
if event:
return UsageEvent.from_event(event)
return None
16 changes: 8 additions & 8 deletions src/posit/connect/metrics/visits.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ class Visits(Resources):
@overload
def find(
self,
*,
content_guid: str = ...,
min_data_version: int = ...,
start: str = ...,
Expand All @@ -119,7 +120,7 @@ def find(
...

@overload
def find(self, *args, **kwargs) -> List[VisitEvent]:
def find(self, **kwargs) -> List[VisitEvent]:
"""Find visits.
Returns
Expand All @@ -128,15 +129,14 @@ def find(self, *args, **kwargs) -> List[VisitEvent]:
"""
...

def find(self, *args, **kwargs) -> List[VisitEvent]:
def find(self, **kwargs) -> List[VisitEvent]:
"""Find visits.
Returns
-------
List[Visit]
"""
params = dict(*args, **kwargs)
params = rename_params(params)
params = rename_params(kwargs)

path = "/v1/instrumentation/content/visits"
url = self.url + path
Expand All @@ -153,6 +153,7 @@ def find(self, *args, **kwargs) -> List[VisitEvent]:
@overload
def find_one(
self,
*,
content_guid: str = ...,
min_data_version: int = ...,
start: str = ...,
Expand All @@ -178,7 +179,7 @@ def find_one(
...

@overload
def find_one(self, *args, **kwargs) -> VisitEvent | None:
def find_one(self, **kwargs) -> VisitEvent | None:
"""Find a visit.
Returns
Expand All @@ -187,15 +188,14 @@ def find_one(self, *args, **kwargs) -> VisitEvent | None:
"""
...

def find_one(self, *args, **kwargs) -> VisitEvent | None:
def find_one(self, **kwargs) -> VisitEvent | None:
"""Find a visit.
Returns
-------
Visit | None
"""
params = dict(*args, **kwargs)
params = rename_params(params)
params = rename_params(kwargs)
path = "/v1/instrumentation/content/visits"
url = self.url + path
paginator = CursorPaginator(self.session, url, params=params)
Expand Down
Loading

0 comments on commit 84371b0

Please sign in to comment.