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

fix: use correct args and kwargs definitions #256

Merged
merged 1 commit into from
Aug 9, 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
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,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these *args additions just for forwards / backwards compatibility?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See below first. This is the equivalent to *, except in this case we have to include *args to match the update signature.

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,
*,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question about the places we're adding *

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to say, all arguments after * must be provided as keyword arguments. So, the user must call find_one(content_guid="asdf"). find_one("asdf") will throw an error.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what we want since positional argument can change over time.

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
Loading