chore: attempt to be more informative for missing attributes

JohnVillalovos · JohnVillalovos · commit 1839c9e79891 · 2021-11-30T08:35:58.000-08:00
A commonly reported issue from users on Gitter is that they get an AttributeError for an attribute that should be present. This is often caused due to the fact that they used the `list()` method to retrieve the object and objects retrieved this way often only have a subset of the full data. Add more details in the AttributeError message that explains the situation to users. This will hopefully allow them to resolve the issue. Update the FAQ in the docs to add a section discussing the issue. Closes #1138
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
