From 212ddfc9e9c5de50d2507cc637c01ceb31aaba41 Mon Sep 17 00:00:00 2001 From: John Villalovos Date: Mon, 27 Jun 2022 12:53:49 -0700 Subject: [PATCH] refactor: avoid possible breaking change in iterator (#2107) Commit b6447211754e126f64e12fc735ad74fe557b7fb4 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. --- gitlab/mixins.py | 7 +++++-- gitlab/utils.py | 3 ++- gitlab/v4/objects/artifacts.py | 14 ++++++++++---- gitlab/v4/objects/files.py | 7 +++++-- gitlab/v4/objects/jobs.py | 19 +++++++++++++------ gitlab/v4/objects/packages.py | 7 +++++-- gitlab/v4/objects/projects.py | 7 +++++-- gitlab/v4/objects/repositories.py | 14 ++++++++++---- gitlab/v4/objects/snippets.py | 14 ++++++++++---- tests/unit/test_utils.py | 2 +- 10 files changed, 66 insertions(+), 28 deletions(-) diff --git a/gitlab/mixins.py b/gitlab/mixins.py index 3c897d43d..519e83f1e 100644 --- a/gitlab/mixins.py +++ b/gitlab/mixins.py @@ -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. @@ -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): diff --git a/gitlab/utils.py b/gitlab/utils.py index 6acb86160..4d2ec8d60 100644 --- a/gitlab/utils.py +++ b/gitlab/utils.py @@ -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) diff --git a/gitlab/v4/objects/artifacts.py b/gitlab/v4/objects/artifacts.py index f5f106d8b..b4a4c0e7d 100644 --- a/gitlab/v4/objects/artifacts.py +++ b/gitlab/v4/objects/artifacts.py @@ -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. @@ -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") @@ -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 @@ -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 + ) diff --git a/gitlab/v4/objects/files.py b/gitlab/v4/objects/files.py index 2fd79fd54..d81b7111b 100644 --- a/gitlab/v4/objects/files.py +++ b/gitlab/v4/objects/files.py @@ -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. @@ -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) diff --git a/gitlab/v4/objects/jobs.py b/gitlab/v4/objects/jobs.py index 3fcd6674e..54c4f3311 100644 --- a/gitlab/v4/objects/jobs.py +++ b/gitlab/v4/objects/jobs.py @@ -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. @@ -147,7 +148,9 @@ 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) @@ -155,9 +158,10 @@ 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. @@ -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. @@ -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) diff --git a/gitlab/v4/objects/packages.py b/gitlab/v4/objects/packages.py index a82080167..50295e040 100644 --- a/gitlab/v4/objects/packages.py +++ b/gitlab/v4/objects/packages.py @@ -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. @@ -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): diff --git a/gitlab/v4/objects/projects.py b/gitlab/v4/objects/projects.py index b21ea3a0a..e912a280f 100644 --- a/gitlab/v4/objects/projects.py +++ b/gitlab/v4/objects/projects.py @@ -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. @@ -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) diff --git a/gitlab/v4/objects/repositories.py b/gitlab/v4/objects/repositories.py index 1f10473aa..e968a6a6a 100644 --- a/gitlab/v4/objects/repositories.py +++ b/gitlab/v4/objects/repositories.py @@ -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. @@ -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) @@ -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. @@ -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) diff --git a/gitlab/v4/objects/snippets.py b/gitlab/v4/objects/snippets.py index aa46c7747..648def296 100644 --- a/gitlab/v4/objects/snippets.py +++ b/gitlab/v4/objects/snippets.py @@ -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. @@ -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): @@ -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. @@ -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): diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 74b48ae31..ce2e776c1 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -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()