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") 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 c2d088204..160ab7ab9 100644 --- a/gitlab/utils.py +++ b/gitlab/utils.py @@ -1,3 +1,4 @@ +import dataclasses import email.message import logging import pathlib @@ -177,6 +178,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,9 +198,17 @@ 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, ) + + +@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])) 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",