Feat/merge trains additional #2547 by isaac-philip · Pull Request #3381 · python-gitlab/python-gitlab

I was reviewing this with the help of Claude Code. I ended up coming up with this.

The nesting of ProjectMergeTrainMergeRequestManager under ProjectMergeTrain is misleading because the merge train's ID is never used in the URL. Compare with the established pattern in this codebase:

# ProjectDeploymentMergeRequest — deployment id IS in the path:
_path = "/projects/{project_id}/deployments/{deployment_id}/merge_requests"
_from_parent_attrs = {"deployment_id": "id", "project_id": "project_id"}

# ProjectMergeRequestDiffVersion — MR iid IS in the path:
_path = "/projects/{project_id}/merge_requests/{mr_iid}/versions"
_from_parent_attrs = {"project_id": "project_id", "mr_iid": "iid"}

In both cases _from_parent_attrs includes the parent's own ID so it genuinely scopes the child resource. Here, _from_parent_attrs = {"project_id": "project_id"} only borrows project_id — the merge train's id is silently discarded. This means:

project.merge_trains.get(1,   lazy=True).merge_requests.get(5)  # GET /projects/1/merge_trains/merge_requests/5
project.merge_trains.get(999, lazy=True).merge_requests.get(5)  # GET /projects/1/merge_trains/merge_requests/5  (identical)

The 1 vs 999 makes no difference. A user would reasonably expect the merge train ID to matter.

The GitLab API endpoint /projects/:id/merge_trains/merge_requests/:iid is a project-level resource, not scoped to a specific merge train. The consistent approach would be to attach the manager directly to Project, alongside merge_trains:

# projects.py
merge_trains: ProjectMergeTrainManager
merge_train_merge_requests: ProjectMergeTrainMergeRequestManager

# merge_trains.py
class ProjectMergeTrainMergeRequestManager(...):
    _path = "/projects/{project_id}/merge_trains/merge_requests"
    _from_parent_attrs = {"project_id": "id"}  # "id" from Project directly

Usage becomes project.merge_train_merge_requests.get(mr_iid) — accurate and consistent with the rest of the codebase.

What do you think @isaac-philip ?