-
Notifications
You must be signed in to change notification settings - Fork 675
Feat/merge trains additional #2547 #3381
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,7 +7,7 @@ | |||||||||||||
| import textwrap | ||||||||||||||
| from collections.abc import Iterable | ||||||||||||||
| from types import ModuleType | ||||||||||||||
| from typing import Any, ClassVar, Generic, TYPE_CHECKING, TypeVar | ||||||||||||||
| from typing import Any, cast, ClassVar, Generic, TYPE_CHECKING, TypeVar | ||||||||||||||
|
|
||||||||||||||
| import gitlab | ||||||||||||||
| from gitlab import types as g_types | ||||||||||||||
|
|
@@ -351,6 +351,7 @@ class RESTManager(Generic[TObjCls]): | |||||||||||||
| _path: ClassVar[str] | ||||||||||||||
| _obj_cls: type[TObjCls] | ||||||||||||||
| _from_parent_attrs: dict[str, Any] = {} | ||||||||||||||
| _parent_ref_attr: ClassVar[str | None] = None | ||||||||||||||
| _types: dict[str, type[g_types.GitlabAttribute]] = {} | ||||||||||||||
|
|
||||||||||||||
| _computed_path: str | ||||||||||||||
|
|
@@ -389,6 +390,18 @@ def _compute_path(self, path: str | None = None) -> str: | |||||||||||||
| self._parent_attrs = data | ||||||||||||||
| return path.format(**data) | ||||||||||||||
|
|
||||||||||||||
| def _get_parent_ref_id(self) -> int | str | None: | ||||||||||||||
| if self._parent is None or not self._parent_ref_attr: | ||||||||||||||
| return None | ||||||||||||||
| if not hasattr(self._parent, self._parent_ref_attr): | ||||||||||||||
| return None | ||||||||||||||
| parent_ref = getattr(self._parent, self._parent_ref_attr) | ||||||||||||||
| if parent_ref is None: | ||||||||||||||
| return None | ||||||||||||||
| if hasattr(parent_ref, "iid"): | ||||||||||||||
| return cast(int, parent_ref.iid) | ||||||||||||||
|
Comment on lines
+401
to
+402
|
||||||||||||||
| if hasattr(parent_ref, "iid"): | |
| return cast(int, parent_ref.iid) | |
| if isinstance(parent_ref, dict) and "iid" in parent_ref: | |
| return parent_ref["iid"] | |
| if hasattr(parent_ref, "iid"): | |
| return parent_ref.iid |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -72,11 +72,14 @@ class GetMixin(HeadMixin[base.TObjCls]): | |||||||||||||||||||||
| _optional_get_attrs: tuple[str, ...] = () | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| @exc.on_http_error(exc.GitlabGetError) | ||||||||||||||||||||||
| def get(self, id: str | int, lazy: bool = False, **kwargs: Any) -> base.TObjCls: | ||||||||||||||||||||||
| def get( | ||||||||||||||||||||||
| self, id: str | int | None = None, lazy: bool = False, **kwargs: Any | ||||||||||||||||||||||
| ) -> base.TObjCls: | ||||||||||||||||||||||
| """Retrieve a single object. | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| Args: | ||||||||||||||||||||||
| id: ID of the object to retrieve | ||||||||||||||||||||||
| id: ID of the object to retrieve. If not provided, falls back to | ||||||||||||||||||||||
| _parent_ref_id from the parent object (if available). | ||||||||||||||||||||||
| lazy: If True, don't request the server, but create a | ||||||||||||||||||||||
| shallow object giving access to the managers. This is | ||||||||||||||||||||||
| useful if you want to avoid useless calls to the API. | ||||||||||||||||||||||
|
Comment on lines
+82
to
85
|
||||||||||||||||||||||
| _parent_ref_id from the parent object (if available). | |
| lazy: If True, don't request the server, but create a | |
| shallow object giving access to the managers. This is | |
| useful if you want to avoid useless calls to the API. | |
| the parent reference resolved by ``self._get_parent_ref_id()`` | |
| when available (for example, when ``_parent_ref_attr`` is set | |
| on the manager). | |
| lazy: If True, don't request the server, but create a shallow | |
| object giving access to the managers. This is useful if you | |
| want to avoid useless calls to the API. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,15 +3,17 @@ | |
| https://docs.gitlab.com/ee/api/merge_trains.html | ||
| """ | ||
|
|
||
| from copy import deepcopy | ||
|
|
||
| import pytest | ||
| import responses | ||
|
|
||
| from gitlab.v4.objects import ProjectMergeTrain | ||
| from gitlab.v4.objects import ProjectMergeTrain, ProjectMergeTrainMergeRequest | ||
|
|
||
| mr_content = { | ||
| "id": 110, | ||
| "merge_request": { | ||
| "id": 1, | ||
| "id": 273, | ||
| "iid": 1, | ||
| "project_id": 3, | ||
| "title": "Test merge train", | ||
|
|
@@ -46,6 +48,10 @@ | |
| "duration": 70, | ||
| } | ||
|
|
||
| merge_train_update = deepcopy(mr_content) | ||
| merge_train_update["merge_request"]["iid"] = 4 | ||
| merge_train_update["pipeline"]["sha"] = "ef33a3zxc3" | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def resp_list_merge_trains(): | ||
|
|
@@ -60,7 +66,62 @@ def resp_list_merge_trains(): | |
| yield rsps | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def resp_merge_trains_merge_request_get(): | ||
| with responses.RequestsMock() as rsps: | ||
| rsps.add( | ||
| method=responses.GET, | ||
| url="http://localhost/api/v4/projects/1/merge_trains/merge_requests/1", | ||
| json=mr_content, | ||
| content_type="application/json", | ||
| status=200, | ||
| ) | ||
| yield rsps | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def resp_merge_trains_merge_request_post(): | ||
| with responses.RequestsMock() as rsps: | ||
| rsps.add( | ||
| method=responses.POST, | ||
| url="http://localhost/api/v4/projects/1/merge_trains/merge_requests/4", | ||
| json=[merge_train_update], | ||
| content_type="application/json", | ||
| status=200, | ||
|
isaac-philip marked this conversation as resolved.
|
||
| match=[responses.matchers.json_params_matcher({"sha": "ef33a3zxc3"})], | ||
| ) | ||
| yield rsps | ||
|
|
||
|
|
||
| def test_list_project_merge_requests(project, resp_list_merge_trains): | ||
| merge_trains = project.merge_trains.list() | ||
| assert isinstance(merge_trains[0], ProjectMergeTrain) | ||
| assert merge_trains[0].id == mr_content["id"] | ||
|
|
||
|
|
||
| def test_merge_trains_status_merge_request( | ||
| project, resp_merge_trains_merge_request_get | ||
| ): | ||
| merge_train_mr: ProjectMergeTrainMergeRequest = project.merge_trains.get( | ||
| 1, lazy=True | ||
| ).merge_requests.get(1) | ||
| assert isinstance(merge_train_mr, ProjectMergeTrainMergeRequest) | ||
| assert merge_train_mr.get_id() == 110 | ||
| assert merge_train_mr.merge_request["iid"] == mr_content["merge_request"]["iid"] | ||
| assert merge_train_mr.pipeline.get("status") == mr_content["pipeline"]["status"] | ||
|
|
||
|
|
||
| def test_merge_train_add_merge_request(project, resp_merge_trains_merge_request_post): | ||
| merge_train: ProjectMergeTrain = project.merge_trains.get(1, lazy=True) | ||
| merge_requests_update = merge_train.merge_requests.update( | ||
| 4, new_data={"sha": "ef33a3zxc3"} | ||
| ) | ||
| assert isinstance(merge_train, ProjectMergeTrain) | ||
| assert ( | ||
| merge_requests_update[0]["pipeline"]["sha"] | ||
| == merge_train_update["pipeline"]["sha"] | ||
| ) | ||
| assert ( | ||
| merge_requests_update[0]["merge_request"]["iid"] | ||
| == merge_train_update["merge_request"]["iid"] | ||
| ) | ||
|
Comment on lines
+114
to
+127
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really needed? Is it really used? I'd prefer to not add this if it isn't needed. It doesn't seem like the tests (non-mixin ones) actually end up using this.
Or I don't understand the changes very well.