-
Notifications
You must be signed in to change notification settings - Fork 643
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
ProjectMergeRequestApprovalRule.save() throws 404 #2548
Comments
Looks like we have some old code here: We might be able to just get rid of that custom save method, things might be more consistent these days. I don't use Premium features personally, so this needs some digging on why we even have that in the code. |
Indeed the used identifier is incorrect. It would perform a call to /api/v4/projects/1/merge_requests/1/approval_rules/3 instead of /api/v4/projects/1/merge_requests/1/approval_rules/1 when saving. The unit test here does not catch that, since the project ID, merge request ID, and approval rule ID are all 1, so a mix up is not detected. Docs for this feature: |
We're actively encountering this bug ourselves, was there a plan to get #2773 merged? |
It is now merged and a new release of python-gitlab was made. |
Description of the problem, including code/CLI snippet
gl.project.get(proj_id).merge_requests.get(mr_iid).approval_rules.get(rule_id).save()
This example is an MVP example; actually making changes to the rule object before calling
.save()
doesn't change the behaviourExpected Behavior
The function should succeed silently, returning
None
Actual Behavior
gitlab.exceptions.GitlabUpdateError: 404: 404 Not found
is thrown. Trying it with debug mode on, it appears as though the root cause of the issue is that when the CLI invokes/projects/:id/merge_requests/:merge_request_iid/approval_rules/:approval_rule_id
in the API,:id
(i.e. project ID) is passed where the URL expects:approval_rule_id
, as can be seen from this debug output (anonymized to remove sensitive information)Specifications
The text was updated successfully, but these errors were encountered: