From b02411a224f3ac9c7342f7c3037d3d247109c4c8 Mon Sep 17 00:00:00 2001
From: "John L. Villalovos" <john@sodarock.com>
Date: Fri, 21 Jun 2024 16:23:19 -0700
Subject: [PATCH 1/3] chore: use correct type-hint for `die()`
---
gitlab/cli.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/gitlab/cli.py b/gitlab/cli.py
index 1dcb02cab..fa139a7d5 100644
--- a/gitlab/cli.py
+++ b/gitlab/cli.py
@@ -11,6 +11,7 @@
Callable,
cast,
Dict,
+ NoReturn,
Optional,
Tuple,
Type,
@@ -97,7 +98,7 @@ def wrapped_f(*args: Any, **kwargs: Any) -> Any:
return wrap
-def die(msg: str, e: Optional[Exception] = None) -> None:
+def die(msg: str, e: Optional[Exception] = None) -> NoReturn:
if e:
msg = f"{msg} ({e})"
sys.stderr.write(f"{msg}\n")
From 25cdeee604d8604d0f9452d4b895d4775ce4a199 Mon Sep 17 00:00:00 2001
From: "John L. Villalovos" <john@sodarock.com>
Date: Fri, 21 Jun 2024 17:35:50 -0700
Subject: [PATCH 2/3] chore: add `show_caller` argument to `utils.warn()`
This allows us to not add the caller's location to the UserWarning
message.
---
gitlab/utils.py | 5 ++++-
tests/unit/test_utils.py | 21 +++++++++++++++++++++
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/gitlab/utils.py b/gitlab/utils.py
index c2d088204..96cecc016 100644
--- a/gitlab/utils.py
+++ b/gitlab/utils.py
@@ -177,6 +177,7 @@ def warn(
*,
category: Optional[Type[Warning]] = None,
source: Optional[Any] = None,
+ show_caller: bool = True,
) -> None:
"""This `warnings.warn` wrapper function attempts to show the location causing the
warning in the user code that called the library.
@@ -196,8 +197,10 @@ def warn(
frame_dir = str(pathlib.Path(frame.filename).parent.resolve())
if not frame_dir.startswith(str(pg_dir)):
break
+ if show_caller:
+ message += warning_from
warnings.warn(
- message=message + warning_from,
+ message=message,
category=category,
stacklevel=stacklevel,
source=source,
diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
index db030b61f..170f4cc41 100644
--- a/tests/unit/test_utils.py
+++ b/tests/unit/test_utils.py
@@ -123,6 +123,27 @@ def test_warn(self):
assert __file__ in str(warning.message)
assert warn_source == warning.source
+ def test_warn_no_show_caller(self):
+ warn_message = "short and stout"
+ warn_source = "teapot"
+
+ with warnings.catch_warnings(record=True) as caught_warnings:
+ utils.warn(
+ message=warn_message,
+ category=UserWarning,
+ source=warn_source,
+ show_caller=False,
+ )
+ assert len(caught_warnings) == 1
+ warning = caught_warnings[0]
+ # File name is this file as it is the first file outside of the `gitlab/` path.
+ assert __file__ == warning.filename
+ assert warning.category == UserWarning
+ assert isinstance(warning.message, UserWarning)
+ assert warn_message in str(warning.message)
+ assert __file__ not in str(warning.message)
+ assert warn_source == warning.source
+
@pytest.mark.parametrize(
"source,expected",
From e1e00607990316db66ee0aaa1149888b91af4e53 Mon Sep 17 00:00:00 2001
From: "John L. Villalovos" <john@sodarock.com>
Date: Wed, 3 Jul 2024 16:49:32 -0700
Subject: [PATCH 3/3] fix(cli): generate UserWarning if `list` does not return
all entries
Previously in the CLI, calls to `list()` would have `get_all=False` by
default. Therefore hiding the fact that not all items are being
returned if there were more than 20 items.
Added `--no-get-all` option to `list` actions. Along with the already
existing `--get-all`.
Closes: #2900
---
gitlab/client.py | 20 +++++++++++++++++---
gitlab/utils.py | 7 +++++++
gitlab/v4/cli.py | 32 +++++++++++++++++++++++++-------
3 files changed, 49 insertions(+), 10 deletions(-)
diff --git a/gitlab/client.py b/gitlab/client.py
index 9341a7205..3b069b031 100644
--- a/gitlab/client.py
+++ b/gitlab/client.py
@@ -869,6 +869,7 @@ def http_list(
query_data: Optional[Dict[str, Any]] = None,
*,
iterator: Optional[bool] = None,
+ message_details: Optional[utils.WarnMessageData] = None,
**kwargs: Any,
) -> Union["GitlabList", List[Dict[str, Any]]]:
"""Make a GET request to the Gitlab server for list-oriented queries.
@@ -952,16 +953,29 @@ def should_emit_warning() -> bool:
# Warn the user that they are only going to retrieve `per_page`
# maximum items. This is a common cause of issues filed.
total_items = "many" if gl_list.total is None else gl_list.total
- utils.warn(
- message=(
+ if message_details is not None:
+ message = message_details.message.format_map(
+ {
+ "len_items": len(items),
+ "per_page": gl_list.per_page,
+ "total_items": total_items,
+ }
+ )
+ show_caller = message_details.show_caller
+ else:
+ message = (
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"`get_all=False` to the `list()` call."
- ),
+ )
+ show_caller = True
+ utils.warn(
+ message=message,
category=UserWarning,
+ show_caller=show_caller,
)
return items
diff --git a/gitlab/utils.py b/gitlab/utils.py
index 96cecc016..160ab7ab9 100644
--- a/gitlab/utils.py
+++ b/gitlab/utils.py
@@ -1,3 +1,4 @@
+import dataclasses
import email.message
import logging
import pathlib
@@ -205,3 +206,9 @@ def warn(
stacklevel=stacklevel,
source=source,
)
+
+
+@dataclasses.dataclass
+class WarnMessageData:
+ message: str
+ show_caller: bool
diff --git a/gitlab/v4/cli.py b/gitlab/v4/cli.py
index 27f8cf353..8192f9558 100644
--- a/gitlab/v4/cli.py
+++ b/gitlab/v4/cli.py
@@ -1,4 +1,5 @@
import argparse
+import json
import operator
import sys
from typing import Any, Dict, List, Optional, Type, TYPE_CHECKING, Union
@@ -140,8 +141,16 @@ def do_list(
) -> Union[gitlab.base.RESTObjectList, List[gitlab.base.RESTObject]]:
if TYPE_CHECKING:
assert isinstance(self.mgr, gitlab.mixins.ListMixin)
+ message_details = gitlab.utils.WarnMessageData(
+ message=(
+ "Your query returned {len_items} of {total_items} items. To return all "
+ "items use `--get-all`. To silence this warning use `--no-get-all`."
+ ),
+ show_caller=False,
+ )
+
try:
- result = self.mgr.list(**self.args)
+ result = self.mgr.list(**self.args, message_details=message_details)
except Exception as e: # pragma: no cover, cli.die is unit-tested
cli.die("Impossible to list objects", e)
return result
@@ -238,12 +247,25 @@ 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(
+ get_all_group = sub_parser_action.add_mutually_exclusive_group()
+ get_all_group.add_argument(
"--get-all",
required=False,
- action="store_true",
+ action="store_const",
+ const=True,
+ default=None,
+ dest="get_all",
help="Return all items from the server, without pagination.",
)
+ get_all_group.add_argument(
+ "--no-get-all",
+ required=False,
+ action="store_const",
+ const=False,
+ default=None,
+ dest="get_all",
+ help="Don't return all items from the server.",
+ )
if action_name == "delete":
if cls._id_attr is not None:
@@ -416,8 +438,6 @@ def get_dict(
class JSONPrinter:
@staticmethod
def display(d: Union[str, Dict[str, Any]], **_kwargs: Any) -> None:
- import json # noqa
-
print(json.dumps(d))
@staticmethod
@@ -426,8 +446,6 @@ def display_list(
fields: List[str],
**_kwargs: Any,
) -> None:
- import json # noqa
-
print(json.dumps([get_dict(obj, fields) for obj in data]))