API for get and add MR to merge train is missing

[Viatorus]

Description of the problem, including code/CLI snippet

The API for both "new" merge-train features is missing:

• Get the status of a merge request on a merge train
• Add a merge request to a merge train

Could you please add them?

Specifications

• python-gitlab version: 4.4.0
• API version you are using (v3/v4): v4
• Gitlab server version (or gitlab.com): >= 15.11

[Viatorus]

@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!)

[JohnVillalovos]

@Viatorus I took a look at this and after about an hour I am not quite sure how to do it as the GitLab API is a little weird. Hopefully @nejch has some ideas on a clean way to implement this. My main issue was how to set things up so it will make sense for calling it.

[JohnVillalovos]

@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]

[nejch]

Thanks @Viatorus @JohnVillalovos. Just so I understand, we're talking about these 2 endpoints in the issue description:

GET /projects/:id/merge_trains/merge_requests/:merge_request_iid    # Get the status of a merge request on a merge train
POST /projects/:id/merge_trains/merge_requests/:merge_request_iid   # Add a merge request to a merge train

plus in the diff above:

GET /projects/:id/merge_trains/:target_branch                       # List merge requests in a merge train

@JohnVillalovos I like that idea, the only issue is we are then nesting ProjectMergeTrainMergeRequest inside ProjectMergeTrain, for which currently there is no way the user can get() directly, only via a list() call. So the mr_train.merge_request.get() would have to be called on an arbitrary item returned from the parent list() call.

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 Project already might be a good compromise. So combining your approach with having that already as project.merge_train_merge_requests.get(iid) and project.merge_train_merge_requests.create() for example. WDYT? Especially as the two endpoints don't have any dependency on any kind of merge train IDs.

Still not sure how that would work with the merge_trains/:target_branch one though. Maybe a custom project.merge_train_merge_requests.list(branch=branch)?

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! 😅