Skip to content

Commit

Permalink
refactor: avoid possible breaking change in iterator (#2107)
Browse files Browse the repository at this point in the history
Commit b644721 inadvertently
introduced a possible breaking change as it added a new argument
`iterator` and added it in between existing (potentially positional) arguments.

This moves the `iterator` argument to the end of the argument list and
requires it to be a keyword-only argument.
  • Loading branch information
JohnVillalovos committed Jun 27, 2022
1 parent ebd5795 commit 212ddfc
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 28 deletions.
7 changes: 5 additions & 2 deletions gitlab/mixins.py
Expand Up @@ -613,9 +613,10 @@ class DownloadMixin(_RestObjectBase):
def download(
self,
streamed: bool = False,
iterator: bool = False,
action: Optional[Callable] = None,
chunk_size: int = 1024,
*,
iterator: bool = False,
**kwargs: Any,
) -> Optional[Union[bytes, Iterator[Any]]]:
"""Download the archive of a resource export.
Expand Down Expand Up @@ -644,7 +645,9 @@ def download(
)
if TYPE_CHECKING:
assert isinstance(result, requests.Response)
return utils.response_content(result, streamed, iterator, action, chunk_size)
return utils.response_content(
result, streamed, action, chunk_size, iterator=iterator
)


class SubscribableMixin(_RestObjectBase):
Expand Down
3 changes: 2 additions & 1 deletion gitlab/utils.py
Expand Up @@ -34,9 +34,10 @@ def __call__(self, chunk: Any) -> None:
def response_content(
response: requests.Response,
streamed: bool,
iterator: bool,
action: Optional[Callable],
chunk_size: int,
*,
iterator: bool,
) -> Optional[Union[bytes, Iterator[Any]]]:
if iterator:
return response.iter_content(chunk_size=chunk_size)
Expand Down
14 changes: 10 additions & 4 deletions gitlab/v4/objects/artifacts.py
Expand Up @@ -75,9 +75,10 @@ def download(
ref_name: str,
job: str,
streamed: bool = False,
iterator: bool = False,
action: Optional[Callable] = None,
chunk_size: int = 1024,
*,
iterator: bool = False,
**kwargs: Any,
) -> Optional[Union[bytes, Iterator[Any]]]:
"""Get the job artifacts archive from a specific tag or branch.
Expand Down Expand Up @@ -110,7 +111,9 @@ def download(
)
if TYPE_CHECKING:
assert isinstance(result, requests.Response)
return utils.response_content(result, streamed, iterator, action, chunk_size)
return utils.response_content(
result, streamed, action, chunk_size, iterator=iterator
)

@cli.register_custom_action(
"ProjectArtifactManager", ("ref_name", "artifact_path", "job")
Expand All @@ -122,9 +125,10 @@ def raw(
artifact_path: str,
job: str,
streamed: bool = False,
iterator: bool = False,
action: Optional[Callable] = None,
chunk_size: int = 1024,
*,
iterator: bool = False,
**kwargs: Any,
) -> Optional[Union[bytes, Iterator[Any]]]:
"""Download a single artifact file from a specific tag or branch from
Expand Down Expand Up @@ -158,4 +162,6 @@ def raw(
)
if TYPE_CHECKING:
assert isinstance(result, requests.Response)
return utils.response_content(result, streamed, iterator, action, chunk_size)
return utils.response_content(
result, streamed, action, chunk_size, iterator=iterator
)
7 changes: 5 additions & 2 deletions gitlab/v4/objects/files.py
Expand Up @@ -230,9 +230,10 @@ def raw(
file_path: str,
ref: str,
streamed: bool = False,
iterator: bool = False,
action: Optional[Callable[..., Any]] = None,
chunk_size: int = 1024,
*,
iterator: bool = False,
**kwargs: Any,
) -> Optional[Union[bytes, Iterator[Any]]]:
"""Return the content of a file for a commit.
Expand Down Expand Up @@ -265,7 +266,9 @@ def raw(
)
if TYPE_CHECKING:
assert isinstance(result, requests.Response)
return utils.response_content(result, streamed, iterator, action, chunk_size)
return utils.response_content(
result, streamed, action, chunk_size, iterator=iterator
)

@cli.register_custom_action("ProjectFileManager", ("file_path", "ref"))
@exc.on_http_error(exc.GitlabListError)
Expand Down
19 changes: 13 additions & 6 deletions gitlab/v4/objects/jobs.py
Expand Up @@ -116,9 +116,10 @@ def delete_artifacts(self, **kwargs: Any) -> None:
def artifacts(
self,
streamed: bool = False,
iterator: bool = False,
action: Optional[Callable[..., Any]] = None,
chunk_size: int = 1024,
*,
iterator: bool = False,
**kwargs: Any,
) -> Optional[Union[bytes, Iterator[Any]]]:
"""Get the job artifacts.
Expand Down Expand Up @@ -147,17 +148,20 @@ def artifacts(
)
if TYPE_CHECKING:
assert isinstance(result, requests.Response)
return utils.response_content(result, streamed, iterator, action, chunk_size)
return utils.response_content(
result, streamed, action, chunk_size, iterator=iterator
)

@cli.register_custom_action("ProjectJob")
@exc.on_http_error(exc.GitlabGetError)
def artifact(
self,
path: str,
streamed: bool = False,
iterator: bool = False,
action: Optional[Callable[..., Any]] = None,
chunk_size: int = 1024,
*,
iterator: bool = False,
**kwargs: Any,
) -> Optional[Union[bytes, Iterator[Any]]]:
"""Get a single artifact file from within the job's artifacts archive.
Expand Down Expand Up @@ -187,16 +191,19 @@ def artifact(
)
if TYPE_CHECKING:
assert isinstance(result, requests.Response)
return utils.response_content(result, streamed, iterator, action, chunk_size)
return utils.response_content(
result, streamed, action, chunk_size, iterator=iterator
)

@cli.register_custom_action("ProjectJob")
@exc.on_http_error(exc.GitlabGetError)
def trace(
self,
streamed: bool = False,
iterator: bool = False,
action: Optional[Callable[..., Any]] = None,
chunk_size: int = 1024,
*,
iterator: bool = False,
**kwargs: Any,
) -> Dict[str, Any]:
"""Get the job trace.
Expand Down Expand Up @@ -226,7 +233,7 @@ def trace(
if TYPE_CHECKING:
assert isinstance(result, requests.Response)
return_value = utils.response_content(
result, streamed, iterator, action, chunk_size
result, streamed, action, chunk_size, iterator=iterator
)
if TYPE_CHECKING:
assert isinstance(return_value, dict)
Expand Down
7 changes: 5 additions & 2 deletions gitlab/v4/objects/packages.py
Expand Up @@ -103,9 +103,10 @@ def download(
package_version: str,
file_name: str,
streamed: bool = False,
iterator: bool = False,
action: Optional[Callable] = None,
chunk_size: int = 1024,
*,
iterator: bool = False,
**kwargs: Any,
) -> Optional[Union[bytes, Iterator[Any]]]:
"""Download a generic package.
Expand Down Expand Up @@ -135,7 +136,9 @@ def download(
result = self.gitlab.http_get(path, streamed=streamed, raw=True, **kwargs)
if TYPE_CHECKING:
assert isinstance(result, requests.Response)
return utils.response_content(result, streamed, iterator, action, chunk_size)
return utils.response_content(
result, streamed, action, chunk_size, iterator=iterator
)


class GroupPackage(RESTObject):
Expand Down
7 changes: 5 additions & 2 deletions gitlab/v4/objects/projects.py
Expand Up @@ -476,9 +476,10 @@ def snapshot(
self,
wiki: bool = False,
streamed: bool = False,
iterator: bool = False,
action: Optional[Callable] = None,
chunk_size: int = 1024,
*,
iterator: bool = False,
**kwargs: Any,
) -> Optional[Union[bytes, Iterator[Any]]]:
"""Return a snapshot of the repository.
Expand Down Expand Up @@ -508,7 +509,9 @@ def snapshot(
)
if TYPE_CHECKING:
assert isinstance(result, requests.Response)
return utils.response_content(result, streamed, iterator, action, chunk_size)
return utils.response_content(
result, streamed, action, chunk_size, iterator=iterator
)

@cli.register_custom_action("Project", ("scope", "search"))
@exc.on_http_error(exc.GitlabSearchError)
Expand Down
14 changes: 10 additions & 4 deletions gitlab/v4/objects/repositories.py
Expand Up @@ -107,9 +107,10 @@ def repository_raw_blob(
self,
sha: str,
streamed: bool = False,
iterator: bool = False,
action: Optional[Callable[..., Any]] = None,
chunk_size: int = 1024,
*,
iterator: bool = False,
**kwargs: Any,
) -> Optional[Union[bytes, Iterator[Any]]]:
"""Return the raw file contents for a blob.
Expand Down Expand Up @@ -139,7 +140,9 @@ def repository_raw_blob(
)
if TYPE_CHECKING:
assert isinstance(result, requests.Response)
return utils.response_content(result, streamed, iterator, action, chunk_size)
return utils.response_content(
result, streamed, action, chunk_size, iterator=iterator
)

@cli.register_custom_action("Project", ("from_", "to"))
@exc.on_http_error(exc.GitlabGetError)
Expand Down Expand Up @@ -195,10 +198,11 @@ def repository_archive(
self,
sha: str = None,
streamed: bool = False,
iterator: bool = False,
action: Optional[Callable[..., Any]] = None,
chunk_size: int = 1024,
format: Optional[str] = None,
*,
iterator: bool = False,
**kwargs: Any,
) -> Optional[Union[bytes, Iterator[Any]]]:
"""Return an archive of the repository.
Expand Down Expand Up @@ -234,7 +238,9 @@ def repository_archive(
)
if TYPE_CHECKING:
assert isinstance(result, requests.Response)
return utils.response_content(result, streamed, iterator, action, chunk_size)
return utils.response_content(
result, streamed, action, chunk_size, iterator=iterator
)

@cli.register_custom_action("Project")
@exc.on_http_error(exc.GitlabDeleteError)
Expand Down
14 changes: 10 additions & 4 deletions gitlab/v4/objects/snippets.py
Expand Up @@ -29,9 +29,10 @@ class Snippet(UserAgentDetailMixin, SaveMixin, ObjectDeleteMixin, RESTObject):
def content(
self,
streamed: bool = False,
iterator: bool = False,
action: Optional[Callable[..., Any]] = None,
chunk_size: int = 1024,
*,
iterator: bool = False,
**kwargs: Any,
) -> Optional[Union[bytes, Iterator[Any]]]:
"""Return the content of a snippet.
Expand Down Expand Up @@ -60,7 +61,9 @@ def content(
)
if TYPE_CHECKING:
assert isinstance(result, requests.Response)
return utils.response_content(result, streamed, iterator, action, chunk_size)
return utils.response_content(
result, streamed, action, chunk_size, iterator=iterator
)


class SnippetManager(CRUDMixin, RESTManager):
Expand Down Expand Up @@ -106,9 +109,10 @@ class ProjectSnippet(UserAgentDetailMixin, SaveMixin, ObjectDeleteMixin, RESTObj
def content(
self,
streamed: bool = False,
iterator: bool = False,
action: Optional[Callable[..., Any]] = None,
chunk_size: int = 1024,
*,
iterator: bool = False,
**kwargs: Any,
) -> Optional[Union[bytes, Iterator[Any]]]:
"""Return the content of a snippet.
Expand Down Expand Up @@ -137,7 +141,9 @@ def content(
)
if TYPE_CHECKING:
assert isinstance(result, requests.Response)
return utils.response_content(result, streamed, iterator, action, chunk_size)
return utils.response_content(
result, streamed, action, chunk_size, iterator=iterator
)


class ProjectSnippetManager(CRUDMixin, RESTManager):
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/test_utils.py
Expand Up @@ -37,7 +37,7 @@ def test_response_content(capsys):

resp = requests.get("https://example.com", stream=True)
utils.response_content(
resp, streamed=True, iterator=False, action=None, chunk_size=1024
resp, streamed=True, action=None, chunk_size=1024, iterator=False
)

captured = capsys.readouterr()
Expand Down

0 comments on commit 212ddfc

Please sign in to comment.