Skip to content

ProjectMergeRequest.merge fails because of incorrect request behaviour #1120

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

Closed
ferhat-aram opened this issue Jun 19, 2020 · 9 comments
Closed

Comments

@ferhat-aram
Copy link

ferhat-aram commented Jun 19, 2020

Problem

When I'm trying to merge a merge request via the ProjectMergeRequest.merge function and with "should_remove_source_branch" and/or "merge_when_pipeline_succeeds", I get the HTTP Code 405 with the message Method not allowed back.

Expected Behavior

I tried to do a API-Call as explained here https://docs.gitlab.com/ee/api/merge_requests.html#accept-mr and it works.

Using curl and sending a put request to /projects/:id/merge_requests/:merge_request_iid/merge?remove_source_branch=True worked fine.

Actual Behavior

The problem here is, that pythlon_gitlab merge API doesn't generate a query string, but send the paramters as post_data, which isn't accepted by the GitLab API.

I tried a curl request with "Content-type: application/json" -d '{"merge_when_pipeline_succeeds": "True"}' and /projects/:id/merge_requests/:merge_request_iid/merge and the request failed.

It's very clear, that the request itself isn't done properly as expected (File :

gitlab/v4/objects.py

        path = "%s/%s/merge" % (self.manager.path, self.get_id())
        data = {}
        if merge_commit_message:
            data["merge_commit_message"] = merge_commit_message
        if should_remove_source_branch:
            data["should_remove_source_branch"] = True
        if merge_when_pipeline_succeeds:
            data["merge_when_pipeline_succeeds"] = True

        server_data = self.manager.gitlab.http_put(path, post_data=data, **kwargs)
        self._update_attrs(server_data)

The code handles those two parameters as post_data, while it should handle them as query_data:

gitlab/init.py

        query_data = query_data or {} #Expected: query_data["merge_when_pipeline_succeeds"]=True, Actual: None
        post_data = post_data or {} #Expected: None, Actual: post_data["merge_when_pipeline_succeeds"]=True

Specifications

  • package version: 2.3.1
  • API version: v4
  • Gitlab server version (or gitlab.com): 12.9.3-ee
@ferhat-aram
Copy link
Author

I've already created a branch locally, but I have no rights to push a new branch, sadly.
Could you please allow me access?

@max-wittig
Copy link
Member

@ferhat-aram Python-gitlab is just a wrapper around the GitLab API. I can't help you with your permission issues.

@ferhat-aram
Copy link
Author

ferhat-aram commented Jun 19, 2020

Yeah I know that, but the Wrapper itself does this and not the GitLab API -> The GitLab API works completley fine here. I already tested it several times via curl and as I already said, I fixed it locally by changing the handling of the should_remove_source_branch and merge_when_pipeline_succeeds as query_data instead of post_data

So the problem here relates to the wrapper itself and not the actual API.

@ferhat-aram
Copy link
Author

This problem isn't something new. I've got to do that, after it was broken for several months. I guess the GitLab API changed at one time and the wrapper wasn't updated accordingly to those API changes.

@max-wittig
Copy link
Member

Ah yes. You can fork the project and create a merge request: https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request

@nejch
Copy link
Member

nejch commented Jun 19, 2020

What's weird is that this works for me as is, currently. I have pipelines with scripts using this from today where I've had no issues. Example (I have no idea why GitLab provides both remove_source_branch on create and should_remove_source_branch on accept, but I use both just in case):

    mr = downstream.mergerequests.create({'source_branch': source_branch,
                                          'target_branch': target_branch,
                                          'assignee_id': assignee,
                                          'title': title,
                                          'description': description,
                                          'remove_source_branch': 'true'
                                          })

    time.sleep(10) # wait for sidekiq
    print('[Info] Applying MR merge settings..')
    mr.merge(should_remove_source_branch=True,
             merge_when_pipeline_succeeds=True)

So I'm wondering - are you getting this consistently on a clean open MR (previous API calls can affect that)? can you post the code and the logs? You can get 405 for a bunch of reasons (Work in Progress, Closed, Pipeline Pending Completion, or Failed while requiring Success). I've also seen this when I try to apply the merge too quickly after creation (hence the hardcoded wait above). I'll check if my existing project config or the remove_source_branch on creation means it's just a fluke that this works for me.

@ferhat-aram
Copy link
Author

ferhat-aram commented Jun 19, 2020

The request generally doesn't work for me with post data. I tried it several times. What GitLab version do you use?

@nejch
Copy link
Member

nejch commented Jun 20, 2020

I just tried this on gitlab.com (13.1.0-pre) on an open MR with an in-progress pipeline, and this works (merge is set for after pipeline succeeds and branch is set for deletion, and the MR is eventually merged).

curl -s -X PUT -H "Private-Token: $GL_TOKEN" \
    -H "Content-type: application/json" \
    -d '{"merge_when_pipeline_succeeds": "true", "should_remove_source_branch": "true"}' \
    https://gitlab.com/api/v4/projects/$PROJECT_ID/merge_requests/$MR_ID/merge

I also tried the curl call without the data and got a 405 response (with the project configured to require pipelines to succeed before merge). That's why I was wondering if your API calls were hitting one of the conditions that trigger 405 as described above. But maybe this fixes some other quirks, like reverse proxies that mess with request bodies or something.

@nejch
Copy link
Member

nejch commented Sep 6, 2020

Since the PR was merged I guess this can be closed :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants