Merge pull request #1702 from python-gitlab/jlvillal/attribute_help

nejch · web-flow · commit 387e59fda12c · 2021-12-01T01:32:57.000+01:00
chore: attempt to be more informative for missing attributes
diff --git a/docs/faq.rst b/docs/faq.rst
@@ -16,6 +16,18 @@ I cannot edit the merge request / issue I've just retrieved
     See the :ref:`merge requests example <merge_requests_examples>` and the
     :ref:`issues examples <issues_examples>`.
 
+.. _attribute_error_list:
+
+I get an ``AttributeError`` when accessing attributes of an object retrieved via a ``list()`` call.
+    Fetching a list of objects, doesn’t always include all attributes in the
+    objects. To retrieve an object with all attributes use a ``get()`` call.
+
+    Example with projects::
+
+        for projects in gl.projects.list():
+            # Retrieve project object with all attributes
+            project = gl.projects.get(project.id)
+
 How can I clone the repository of a project?
     python-gitlab doesn't provide an API to clone a project. You have to use a
     git library or call the ``git`` command.
diff --git a/gitlab/base.py b/gitlab/base.py
@@ -16,9 +16,11 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import importlib
+import textwrap
 from types import ModuleType
 from typing import Any, Dict, Iterable, NamedTuple, Optional, Tuple, Type
 
+import gitlab
 from gitlab import types as g_types
 from gitlab.exceptions import GitlabParsingError
 
@@ -32,6 +34,12 @@
 ]
 
 
+_URL_ATTRIBUTE_ERROR = (
+    f"https://python-gitlab.readthedocs.io/en/{gitlab.__version__}/"
+    f"faq.html#attribute-error-list"
+)
+
+
 class RESTObject(object):
     """Represents an object built from server data.
 
@@ -45,13 +53,20 @@ class RESTObject(object):
 
     _id_attr: Optional[str] = "id"
     _attrs: Dict[str, Any]
+    _created_from_list: bool  # Indicates if object was created from a list() action
     _module: ModuleType
     _parent_attrs: Dict[str, Any]
     _short_print_attr: Optional[str] = None
     _updated_attrs: Dict[str, Any]
     manager: "RESTManager"
 
-    def __init__(self, manager: "RESTManager", attrs: Dict[str, Any]) -> None:
+    def __init__(
+        self,
+        manager: "RESTManager",
+        attrs: Dict[str, Any],
+        *,
+        created_from_list: bool = False,
+    ) -> None:
         if not isinstance(attrs, dict):
             raise GitlabParsingError(
                 "Attempted to initialize RESTObject with a non-dictionary value: "
@@ -64,6 +79,7 @@ def __init__(self, manager: "RESTManager", attrs: Dict[str, Any]) -> None:
                 "_attrs": attrs,
                 "_updated_attrs": {},
                 "_module": importlib.import_module(self.__module__),
+                "_created_from_list": created_from_list,
             }
         )
         self.__dict__["_parent_attrs"] = self.manager.parent_attrs
@@ -106,8 +122,22 @@ def __getattr__(self, name: str) -> Any:
             except KeyError:
                 try:
                     return self.__dict__["_parent_attrs"][name]
-                except KeyError:
-                    raise AttributeError(name)
+                except KeyError as exc:
+                    message = (
+                        f"{type(self).__name__!r} object has no attribute {name!r}"
+                    )
+                    if self._created_from_list:
+                        message = (
+                            f"{message}\n\n"
+                            + textwrap.fill(
+                                f"{self.__class__!r} was created via a list() call and "
+                                f"only a subset of the data may be present. To ensure "
+                                f"all data is present get the object using a "
+                                f"get(object.id) call. For more details, see:"
+                            )
+                            + f"\n\n{_URL_ATTRIBUTE_ERROR}"
+                        )
+                    raise AttributeError(message) from exc
 
     def __setattr__(self, name: str, value: Any) -> None:
         self.__dict__["_updated_attrs"][name] = value
@@ -229,7 +259,7 @@ def __next__(self) -> RESTObject:
 
     def next(self) -> RESTObject:
         data = self._list.next()
-        return self._obj_cls(self.manager, data)
+        return self._obj_cls(self.manager, data, created_from_list=True)
 
     @property
     def current_page(self) -> int:
diff --git a/gitlab/mixins.py b/gitlab/mixins.py
@@ -240,7 +240,7 @@ def list(self, **kwargs: Any) -> Union[base.RESTObjectList, List[base.RESTObject
             assert self._obj_cls is not None
         obj = self.gitlab.http_list(path, **data)
         if isinstance(obj, list):
-            return [self._obj_cls(self, item) for item in obj]
+            return [self._obj_cls(self, item, created_from_list=True) for item in obj]
         else:
             return base.RESTObjectList(self, self._obj_cls, obj)
 
diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py
@@ -90,6 +90,30 @@ def test_instantiate_non_dict(self, fake_gitlab, fake_manager):
         with pytest.raises(gitlab.exceptions.GitlabParsingError):
             FakeObject(fake_manager, ["a", "list", "fails"])
 
+    def test_missing_attribute_does_not_raise_custom(self, fake_gitlab, fake_manager):
+        """Ensure a missing attribute does not raise our custom error message
+        if the RESTObject was not created from a list"""
+        obj = FakeObject(manager=fake_manager, attrs={"foo": "bar"})
+        with pytest.raises(AttributeError) as excinfo:
+            obj.missing_attribute
+        exc_str = str(excinfo.value)
+        assert "missing_attribute" in exc_str
+        assert "was created via a list()" not in exc_str
+        assert base._URL_ATTRIBUTE_ERROR not in exc_str
+
+    def test_missing_attribute_from_list_raises_custom(self, fake_gitlab, fake_manager):
+        """Ensure a missing attribute raises our custom error message if the
+        RESTObject was created from a list"""
+        obj = FakeObject(
+            manager=fake_manager, attrs={"foo": "bar"}, created_from_list=True
+        )
+        with pytest.raises(AttributeError) as excinfo:
+            obj.missing_attribute
+        exc_str = str(excinfo.value)
+        assert "missing_attribute" in exc_str
+        assert "was created via a list()" in exc_str
+        assert base._URL_ATTRIBUTE_ERROR in exc_str
+
     def test_picklability(self, fake_manager):
         obj = FakeObject(fake_manager, {"foo": "bar"})
         original_obj_module = obj._module
