-
Notifications
You must be signed in to change notification settings - Fork 644
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
API for get and add MR to merge train is missing #2858
Comments
@nejch @JohnVillalovos can I ask you for your help here please. I have been trying to understand what needs to be added/changed, but I am not able to understand how REST and all the mixins and CRUDs work together. Can you give me an example, where you would say: this command is very similar? (I know this is all voluntary - thanks for keeping the project alive!) |
@nejch This was my initial attempt at this. But I'm not sure if I am on the correct path on this. Your input would be appreciated. diff --git a/gitlab/v4/objects/merge_trains.py b/gitlab/v4/objects/merge_trains.py
index 9f8e1dff..0fddfddf 100644
--- a/gitlab/v4/objects/merge_trains.py
+++ b/gitlab/v4/objects/merge_trains.py
@@ -1,5 +1,16 @@
+"""
+GitLab API:
+https://docs.gitlab.com/ee/api/merge_trains.html
+"""
+
+from typing import Any, cast, List, Optional, Union
+
+import gitlab
+from gitlab import exceptions as exc
+from gitlab import utils
from gitlab.base import RESTManager, RESTObject
-from gitlab.mixins import ListMixin
+from gitlab.mixins import CreateMixin, GetMixin, ListMixin
+from gitlab.types import RequiredOptional
__all__ = [
"ProjectMergeTrain",
@@ -7,8 +18,28 @@ __all__ = [
]
+class ProjectMergeTrainMergeRequest(RESTObject): ...
+
+
+class ProjectMergeTrainMergeRequestManager(GetMixin, CreateMixin, RESTManager):
+ _path = "/projects/{project_id}/merge_trains/merge_requests"
+ _obj_cls = ProjectMergeTrainMergeRequest
+ _from_parent_attrs = {"project_id": "id"}
+ _create_attrs = RequiredOptional(
+ required=("merge_request_iid",),
+ optional=("sha", "squash", "when_pipeline_succeeds"),
+ )
+
+ def get(
+ self, id: Union[str, int], lazy: bool = False, **kwargs: Any
+ ) -> ProjectMergeTrainMergeRequest:
+ return cast(
+ ProjectMergeTrainMergeRequest, super().get(id=id, lazy=lazy, **kwargs)
+ )
+
+
class ProjectMergeTrain(RESTObject):
- pass
+ merge_request: ProjectMergeTrainMergeRequestManager
class ProjectMergeTrainManager(ListMixin, RESTManager):
@@ -16,3 +47,46 @@ class ProjectMergeTrainManager(ListMixin, RESTManager):
_obj_cls = ProjectMergeTrain
_from_parent_attrs = {"project_id": "id"}
_list_filters = ("scope",)
+
+ @gitlab.cli.register_custom_action(
+ cls_names="ProjectMergeTrainManager",
+ required=("target_branch",),
+ optional=("scope", "sort"),
+ )
+ @exc.on_http_error(exc.GitlabListError)
+ def merge_requests(
+ self,
+ *,
+ target_branch: str,
+ scope: Optional[str] = None,
+ sort: Optional[str],
+ **kwargs: Any,
+ ) -> List[ProjectMergeTrainMergeRequest]:
+ """List all merge requests added to a merge train for the requested target branch.
+
+ Args:
+ scope: Return Merge Trains filtered by the given scope. Available
+ scopes are active (to be merged) and complete (have been
+ merged).
+ all: If True, return all the items, without pagination
+ per_page: Number of items to retrieve per request
+ page: ID of the page to return (starts with page 1)
+ iterator: If set to True and no pagination option is
+ defined, return a generator instead of a list
+ **kwargs: Extra options to send to the server (e.g. sudo)
+
+ Raises:
+ GitlabAuthenticationError: If authentication is not correct
+ GitlabListError: If the server failed to perform the request
+
+ Returns:
+ A list of runners matching the scope.
+ """
+ path = self._path + utils.EncodedId(target_branch)
+ query_data = {}
+ if scope is not None:
+ query_data["scope"] = scope
+ if sort is not None:
+ query_data["sort"] = sort
+ obj = self.gitlab.http_list(path, query_data, **kwargs)
+ return [self._obj_cls(self, item) for item in obj] |
Thanks @Viatorus @JohnVillalovos. Just so I understand, we're talking about these 2 endpoints in the issue description:
plus in the diff above:
@JohnVillalovos I like that idea, the only issue is we are then nesting Last year we had a PR for this already, although it relied too much on custom methods IMO (#2552). But the flat approach of adding the manager in Still not sure how that would work with the Sorry @Viatorus this is indeed a weird one, so we have to discuss some internal approaches here, it's not the easiest to contribute for sure! 😅 |
Description of the problem, including code/CLI snippet
The API for both "new" merge-train features is missing:
Could you please add them?
Specifications
The text was updated successfully, but these errors were encountered: