fix: add get_all param (and --get-all) to allow passing all to API

nejch · JohnVillalovos · commit 7c71d5db1199 · 2022-07-21T15:03:11.000-07:00
diff --git a/docs/api-usage-advanced.rst b/docs/api-usage-advanced.rst
@@ -97,7 +97,7 @@ supplying the ``obey_rate_limit`` argument.
    import requests
 
    gl = gitlab.gitlab(url, token, api_version=4)
-   gl.projects.list(all=True, obey_rate_limit=False)
+   gl.projects.list(get_all=True, obey_rate_limit=False)
 
 If you do not disable the rate-limiting feature, you can supply a custom value
 for ``max_retries``; by default, this is set to 10. To retry without bound when
@@ -110,7 +110,7 @@ throttled, you can set this parameter to -1. This parameter is ignored if
    import requests
 
    gl = gitlab.gitlab(url, token, api_version=4)
-   gl.projects.list(all=True, max_retries=12)
+   gl.projects.list(get_all=True, max_retries=12)
 
 .. warning::
 
@@ -133,7 +133,7 @@ is raised for these errors.
    import requests
 
    gl = gitlab.gitlab(url, token, api_version=4)
-   gl.projects.list(all=True, retry_transient_errors=True)
+   gl.projects.list(get_all=True, retry_transient_errors=True)
 
 The default ``retry_transient_errors`` can also be set on the ``Gitlab`` object
 and overridden by individual API calls.
@@ -143,8 +143,8 @@ and overridden by individual API calls.
    import gitlab
    import requests
    gl = gitlab.gitlab(url, token, api_version=4, retry_transient_errors=True)
-   gl.projects.list(all=True)                               # retries due to default value
-   gl.projects.list(all=True, retry_transient_errors=False) # does not retry
+   gl.projects.list(get_all=True)                               # retries due to default value
+   gl.projects.list(get_all=True, retry_transient_errors=False) # does not retry
 
 Timeout
 -------
diff --git a/docs/api-usage.rst b/docs/api-usage.rst
@@ -110,7 +110,7 @@ Examples:
 
 .. warning::
    Calling ``list()`` without any arguments will by default not return the complete list
-   of items. Use either the ``all=True`` or ``iterator=True`` parameters to get all the
+   of items. Use either the ``get_all=True`` or ``iterator=True`` parameters to get all the
    items when using listing methods. See the :ref:`pagination` section for more
    information.
 
@@ -144,7 +144,7 @@ Some objects also provide managers to access related GitLab resources:
 
    # list the issues for a project
    project = gl.projects.get(1)
-   issues = project.issues.list(all=True)
+   issues = project.issues.list(get_all=True)
 
 python-gitlab allows to send any data to the GitLab server when making queries.
 In case of invalid or missing arguments python-gitlab will raise an exception
@@ -314,13 +314,14 @@ listing methods support the ``page`` and ``per_page`` parameters:
 
    The first page is page 1, not page 0.
 
-By default GitLab does not return the complete list of items. Use the ``all``
+By default GitLab does not return the complete list of items. Use the ``get_all``
 parameter to get all the items when using listing methods:
 
 .. code-block:: python
 
-   all_groups = gl.groups.list(all=True)
-   all_owned_projects = gl.projects.list(owned=True, all=True)
+   all_groups = gl.groups.list(get_all=True)
+
+   all_owned_projects = gl.projects.list(owned=True, get_all=True)
 
 You can define the ``per_page`` value globally to avoid passing it to every
 ``list()`` method call:
diff --git a/docs/cli-examples.rst b/docs/cli-examples.rst
@@ -76,19 +76,19 @@ List all the projects:
 
 .. code-block:: console
 
-   $ gitlab project list --all
+   $ gitlab project list --get-all
 
 List all projects of a group:
 
 .. code-block:: console
 
-   $ gitlab group-project list --all --group-id 1
+   $ gitlab group-project list --get-all --group-id 1
 
 List all projects of a group and its subgroups:
 
 .. code-block:: console
 
-   $ gitlab group-project list --all --include-subgroups true --group-id 1
+   $ gitlab group-project list --get-all --include-subgroups true --group-id 1
 
 Limit to 5 items per request, display the 1st page only
 
diff --git a/docs/faq.rst b/docs/faq.rst
@@ -48,3 +48,6 @@ I get an ``AttributeError`` when accessing attributes after ``save()`` or ``refr
     You are most likely trying to access an attribute that was not returned
     by the server on the second request. Please look at the documentation in
     :ref:`object_attributes` to see how to avoid this.
+
+I passed ``all=True`` (or ``--all`` via the CLI) to the API and I still cannot see all items returned.
+    Use ``get_all=True`` (or ``--get-all`` via the CLI). See :ref:`pagination` for more details.
diff --git a/docs/gl_objects/commits.rst b/docs/gl_objects/commits.rst
@@ -27,13 +27,9 @@ results::
     commits = project.commits.list(ref_name='my_branch')
     commits = project.commits.list(since='2016-01-01T00:00:00Z')
 
-.. note::
+List all commits for a project (see :ref:`pagination`) on all branches:
 
-   The available ``all`` listing argument conflicts with the python-gitlab
-   argument. Use ``query_parameters`` to avoid the conflict::
-
-       commits = project.commits.list(all=True,
-                                      query_parameters={'ref_name': 'my_branch'})
+    commits = project.commits.list(get_all=True, all=True)
 
 Create a commit::
 
diff --git a/docs/gl_objects/groups.rst b/docs/gl_objects/groups.rst
@@ -271,7 +271,7 @@ List only direct group members::
 List the group members recursively (including inherited members through
 ancestor groups)::
 
-    members = group.members_all.list(all=True)
+    members = group.members_all.list(get_all=True)
 
 Get only direct group member::
 
diff --git a/docs/gl_objects/projects.rst b/docs/gl_objects/projects.rst
@@ -40,7 +40,7 @@ Results can also be sorted using the following parameters:
 ::
 
     # List all projects (default 20)
-    projects = gl.projects.list(all=True)
+    projects = gl.projects.list(get_all=True)
     # Archived projects
     projects = gl.projects.list(archived=1)
     # Limit to projects with a defined visibility
@@ -559,7 +559,7 @@ List only direct project members::
 List the project members recursively (including inherited members through
 ancestor groups)::
 
-    members = project.members_all.list(all=True)
+    members = project.members_all.list(get_all=True)
 
 Search project members matching a query string::
 
diff --git a/docs/gl_objects/users.rst b/docs/gl_objects/users.rst
@@ -454,4 +454,5 @@ Get the users activities::
 
     activities = gl.user_activities.list(
         query_parameters={'from': '2018-07-01'},
-        all=True, iterator=True)
+        get_all=True,
+    )
diff --git a/gitlab/client.py b/gitlab/client.py
@@ -857,7 +857,7 @@ def http_list(
         Returns:
             A list of the objects returned by the server. If `iterator` is
             True and no pagination-related arguments (`page`, `per_page`,
-            `all`) are defined then a GitlabList object (generator) is returned
+            `get_all`) are defined then a GitlabList object (generator) is returned
             instead. This object will make API calls when needed to fetch the
             next items from the server.
 
@@ -884,7 +884,13 @@ def http_list(
                 category=DeprecationWarning,
             )
 
-        get_all = kwargs.pop("all", None)
+        # Provide a `get_all`` param to avoid clashes with `all` API attributes.
+        get_all = kwargs.pop("get_all", None)
+
+        if get_all is None:
+            # For now, keep `all` without deprecation.
+            get_all = kwargs.pop("all", None)
+
         url = self._build_url(path)
 
         page = kwargs.get("page")
@@ -902,7 +908,7 @@ def http_list(
 
         def should_emit_warning() -> bool:
             # No warning is emitted if any of the following conditions apply:
-            # * `all=False` was set in the `list()` call.
+            # * `get_all=False` was set in the `list()` call.
             # * `page` was set in the `list()` call.
             # * GitLab did not return the `x-per-page` header.
             # * Number of items received is less than per-page value.
@@ -927,12 +933,12 @@ def should_emit_warning() -> bool:
         total_items = "many" if gl_list.total is None else gl_list.total
         utils.warn(
             message=(
-                f"Calling a `list()` method without specifying `all=True` or "
+                f"Calling a `list()` method without specifying `get_all=True` or "
                 f"`iterator=True` will return a maximum of {gl_list.per_page} items. "
                 f"Your query returned {len(items)} of {total_items} items. See "
                 f"{_PAGINATION_URL} for more details. If this was done intentionally, "
                 f"then this warning can be supressed by adding the argument "
-                f"`all=False` to the `list()` call."
+                f"`get_all=False` to the `list()` call."
             ),
             category=UserWarning,
         )
diff --git a/gitlab/v4/cli.py b/gitlab/v4/cli.py
@@ -240,7 +240,18 @@ def _populate_sub_parser_by_class(
 
             sub_parser_action.add_argument("--page", required=False, type=int)
             sub_parser_action.add_argument("--per-page", required=False, type=int)
-            sub_parser_action.add_argument("--all", required=False, action="store_true")
+            sub_parser_action.add_argument(
+                "--get-all",
+                required=False,
+                action="store_true",
+                help="Return all items from the server, without pagination.",
+            )
+            sub_parser_action.add_argument(
+                "--all",
+                required=False,
+                action="store_true",
+                help="Deprecated. Use --get-all instead.",
+            )
 
         if action_name == "delete":
             if cls._id_attr is not None:
diff --git a/tests/functional/api/test_gitlab.py b/tests/functional/api/test_gitlab.py
@@ -18,7 +18,7 @@ def test_broadcast_messages(gl):
     msg.save()
     msg_id = msg.id
 
-    msg = gl.broadcastmessages.list(all=True)[0]
+    msg = gl.broadcastmessages.list(get_all=True)[0]
     assert msg.color == "#444444"
 
     msg = gl.broadcastmessages.get(msg_id)
@@ -86,13 +86,13 @@ def test_template_dockerfile(gl):
 
 
 def test_template_gitignore(gl):
-    assert gl.gitignores.list(all=True)
+    assert gl.gitignores.list(get_all=True)
     gitignore = gl.gitignores.get("Node")
     assert gitignore.content is not None
 
 
 def test_template_gitlabciyml(gl):
-    assert gl.gitlabciymls.list(all=True)
+    assert gl.gitlabciymls.list(get_all=True)
     gitlabciyml = gl.gitlabciymls.get("Nodejs")
     assert gitlabciyml.content is not None
 
@@ -114,10 +114,10 @@ def test_hooks(gl):
 
 
 def test_namespaces(gl):
-    namespace = gl.namespaces.list(all=True)
+    namespace = gl.namespaces.list(get_all=True)
     assert namespace
 
-    namespace = gl.namespaces.list(search="root", all=True)[0]
+    namespace = gl.namespaces.list(search="root", get_all=True)[0]
     assert namespace.kind == "user"
 
 
@@ -217,8 +217,8 @@ def test_list_all_false_nowarning(gl, recwarn):
 
 
 def test_list_all_true_nowarning(gl, recwarn):
-    """Using `all=True` will disable the warning"""
-    items = gl.gitlabciymls.list(all=True)
+    """Using `get_all=True` will disable the warning"""
+    items = gl.gitlabciymls.list(get_all=True)
     assert not recwarn
     assert len(items) > 20
 
diff --git a/tests/functional/api/test_repository.py b/tests/functional/api/test_repository.py
@@ -105,6 +105,26 @@ def test_create_commit(project):
     assert isinstance(commit.merge_requests(), list)
 
 
+def test_list_all_commits(project):
+    data = {
+        "branch": "new-branch",
+        "start_branch": "main",
+        "commit_message": "New commit on new branch",
+        "actions": [
+            {"action": "create", "file_path": "new-file", "content": "new content"}
+        ],
+    }
+    commit = project.commits.create(data)
+
+    commits = project.commits.list(all=True)
+    assert commit not in commits
+
+    # Listing commits on other branches requires `all` parameter passed to the API
+    all_commits = project.commits.list(get_all=True, all=True)
+    assert commit in all_commits
+    assert len(all_commits) > len(commits)
+
+
 def test_create_commit_status(project):
     commit = project.commits.list()[0]
     status = commit.statuses.create({"state": "success", "sha": commit.id})
diff --git a/tests/functional/api/test_snippets.py b/tests/functional/api/test_snippets.py
@@ -2,7 +2,7 @@
 
 
 def test_snippets(gl):
-    snippets = gl.snippets.list(all=True)
+    snippets = gl.snippets.list(get_all=True)
     assert not snippets
 
     snippet = gl.snippets.create(
@@ -20,7 +20,7 @@ def test_snippets(gl):
     assert snippet.user_agent_detail()["user_agent"]
 
     snippet.delete()
-    assert snippet not in gl.snippets.list(all=True)
+    assert snippet not in gl.snippets.list(get_all=True)
 
 
 def test_project_snippets(project):
diff --git a/tests/functional/cli/test_cli_repository.py b/tests/functional/cli/test_cli_repository.py
@@ -0,0 +1,73 @@
+def test_project_create_file(gitlab_cli, project):
+    file_path = "README"
+    branch = "main"
+    content = "CONTENT"
+    commit_message = "Initial commit"
+
+    cmd = [
+        "project-file",
+        "create",
+        "--project-id",
+        project.id,
+        "--file-path",
+        file_path,
+        "--branch",
+        branch,
+        "--content",
+        content,
+        "--commit-message",
+        commit_message,
+    ]
+    ret = gitlab_cli(cmd)
+
+    assert ret.success
+
+
+def test_list_all_commits(gitlab_cli, project):
+    data = {
+        "branch": "new-branch",
+        "start_branch": "main",
+        "commit_message": "New commit on new branch",
+        "actions": [
+            {"action": "create", "file_path": "new-file", "content": "new content"}
+        ],
+    }
+    commit = project.commits.create(data)
+
+    cmd = ["project-commit", "list", "--project-id", project.id, "--get-all"]
+    ret = gitlab_cli(cmd)
+    assert commit.id not in ret.stdout
+
+    # Listing commits on other branches requires `all` parameter passed to the API
+    cmd = ["project-commit", "list", "--project-id", project.id, "--get-all", "--all"]
+    ret_all = gitlab_cli(cmd)
+    assert commit.id in ret_all.stdout
+    assert len(ret_all.stdout) > len(ret.stdout)
+
+
+def test_revert_commit(gitlab_cli, project):
+    commit = project.commits.list()[0]
+
+    cmd = [
+        "project-commit",
+        "revert",
+        "--project-id",
+        project.id,
+        "--id",
+        commit.id,
+        "--branch",
+        "main",
+    ]
+    ret = gitlab_cli(cmd)
+
+    assert ret.success
+
+
+def test_get_commit_signature_not_found(gitlab_cli, project):
+    commit = project.commits.list()[0]
+
+    cmd = ["project-commit", "signature", "--project-id", project.id, "--id", commit.id]
+    ret = gitlab_cli(cmd)
+
+    assert not ret.success
+    assert "404 Signature Not Found" in ret.stderr
diff --git a/tests/functional/cli/test_cli_v4.py b/tests/functional/cli/test_cli_v4.py
diff --git a/tests/functional/ee-test.py b/tests/functional/ee-test.py
diff --git a/tests/unit/mixins/test_mixin_methods.py b/tests/unit/mixins/test_mixin_methods.py
diff --git a/tests/unit/test_gitlab.py b/tests/unit/test_gitlab.py
diff --git a/tests/unit/test_gitlab_http_methods.py b/tests/unit/test_gitlab_http_methods.py
