Skip to content

Commit 91c4f18

Browse files
igorp-collaboranejch
igorp-collabora
authored and
nejch
committed
feat(api): Make RESTManager generic on RESTObject class
Currently mixins like ListMixin are type hinted to return base RESTObject instead of a specific class like `MergeRequest`. The GetMixin and GetWithoutIdMixin solve this problem by defining a new `get` method for every defined class. However, this creates a lot of duplicated code. Make RESTManager use `typing.Generic` as its base class and use and assign the declared TypeVar to the `_obj_cls` attribute as a type of the passed class. Make both `_obj_cls` and `_path` attributes an abstract properties so that type checkers can check that those attributes were properly defined in subclasses. Mypy will only check then the class is instantiated which makes non-final subclasses possible. Unfortunately pylint will check the declarations not instantiations so add `# pylint: disable=abstract-method` comments to all non-final subclasses like ListMixin. Make `_path` attribute always be `str` instead of sometimes `None`. This eliminates unnecessary type checks. Change all mixins like ListMixin or GetMixin to a subclass. This makes the attribute declarations much cleaner as for example `_list_filters` is now the only attribute defined by ListMixin. Change SidekiqManager to not inherit from RESTManager and only copy its `__init__` method. This is because SidekiqManager never was a real manager and does not define `_path` or `_obj_cls`. Delete `tests/unit/meta/test_ensure_type_hints.py` file as the `get` method is no required to be defined for every class. Signed-off-by: Igor Ponomarev <igor.ponomarev@collabora.com>
1 parent 46dfc50 commit 91c4f18

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

88 files changed

+748
-536
lines changed

‎gitlab/base.py

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,18 @@
44
import pprint
55
import textwrap
66
from types import ModuleType
7-
from typing import Any, Dict, Iterable, Optional, Type, TYPE_CHECKING, Union
7+
from typing import (
8+
Any,
9+
ClassVar,
10+
Dict,
11+
Generic,
12+
Iterable,
13+
Optional,
14+
Type,
15+
TYPE_CHECKING,
16+
TypeVar,
17+
Union,
18+
)
819

920
import gitlab
1021
from gitlab import types as g_types
@@ -48,11 +59,11 @@ class RESTObject:
4859
_repr_attr: Optional[str] = None
4960
_updated_attrs: Dict[str, Any]
5061
_lazy: bool
51-
manager: "RESTManager"
62+
manager: "RESTManager[Any]"
5263

5364
def __init__(
5465
self,
55-
manager: "RESTManager",
66+
manager: "RESTManager[Any]",
5667
attrs: Dict[str, Any],
5768
*,
5869
created_from_list: bool = False,
@@ -269,7 +280,7 @@ class RESTObjectList:
269280
"""
270281

271282
def __init__(
272-
self, manager: "RESTManager", obj_cls: Type[RESTObject], _list: GitlabList
283+
self, manager: "RESTManager[Any]", obj_cls: Type[RESTObject], _list: GitlabList
273284
) -> None:
274285
"""Creates an objects list from a GitlabList.
275286
@@ -335,7 +346,10 @@ def total(self) -> Optional[int]:
335346
return self._list.total
336347

337348

338-
class RESTManager:
349+
TObjCls = TypeVar("TObjCls", bound=RESTObject)
350+
351+
352+
class RESTManager(Generic[TObjCls]):
339353
"""Base class for CRUD operations on objects.
340354
341355
Derived class must define ``_path`` and ``_obj_cls``.
@@ -346,12 +360,12 @@ class RESTManager:
346360

347361
_create_attrs: g_types.RequiredOptional = g_types.RequiredOptional()
348362
_update_attrs: g_types.RequiredOptional = g_types.RequiredOptional()
349-
_path: Optional[str] = None
350-
_obj_cls: Optional[Type[RESTObject]] = None
363+
_path: ClassVar[str]
364+
_obj_cls: type[TObjCls]
351365
_from_parent_attrs: Dict[str, Any] = {}
352366
_types: Dict[str, Type[g_types.GitlabAttribute]] = {}
353367

354-
_computed_path: Optional[str]
368+
_computed_path: str
355369
_parent: Optional[RESTObject]
356370
_parent_attrs: Dict[str, Any]
357371
gitlab: Gitlab
@@ -371,12 +385,10 @@ def __init__(self, gl: Gitlab, parent: Optional[RESTObject] = None) -> None:
371385
def parent_attrs(self) -> Optional[Dict[str, Any]]:
372386
return self._parent_attrs
373387

374-
def _compute_path(self, path: Optional[str] = None) -> Optional[str]:
388+
def _compute_path(self, path: Optional[str] = None) -> str:
375389
self._parent_attrs = {}
376390
if path is None:
377391
path = self._path
378-
if path is None:
379-
return None
380392
if self._parent is None or not self._from_parent_attrs:
381393
return path
382394

@@ -390,5 +402,5 @@ def _compute_path(self, path: Optional[str] = None) -> Optional[str]:
390402
return path.format(**data)
391403

392404
@property
393-
def path(self) -> Optional[str]:
405+
def path(self) -> str:
394406
return self._computed_path

‎gitlab/client.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -397,11 +397,10 @@ def auth(self) -> None:
397397
The `user` attribute will hold a `gitlab.objects.CurrentUser` object on
398398
success.
399399
"""
400-
# pylint: disable=line-too-long
401-
self.user = self._objects.CurrentUserManager(self).get() # type: ignore[assignment]
400+
self.user = self._objects.CurrentUserManager(self).get()
402401

403402
if hasattr(self.user, "web_url") and hasattr(self.user, "username"):
404-
self._check_url(self.user.web_url, path=self.user.username) # type: ignore[union-attr]
403+
self._check_url(self.user.web_url, path=self.user.username)
405404

406405
def version(self) -> Tuple[str, str]:
407406
"""Returns the version and revision of the gitlab server.

0 commit comments

Comments
 (0)