python-gitlab / python-gitlab

A python wrapper for the GitLab API.
https://python-gitlab.readthedocs.io
GNU Lesser General Public License v3.0
2.25k stars 654 forks source link

Cannot update/patch protected branch allowed_to_push/push_access_levels state #2850

Open nickbroon opened 6 months ago

nickbroon commented 6 months ago

Description of the problem, including code/CLI snippet

Breaking out discussion from this Pull Request thread into its own issue.

The Save and Update mixins for protectedbranch doesn't really work for most of it's attributes, as the PATCH/POST api use different values from those return by the API.

The PATCH api requires to send allowed_to_push attribute, but the returned attribute is push_access_levels, as documented in the API and examples: https://docs.gitlab.com/ee/api/protected_branches.html#example-update-a-push_access_level-record

Expected Behavior

To be able to update the `allowed_to_push' GitLab protected branch state.

Actual Behavior

Using the python-gitlab library to modify the push_access_levels attribute that the object has and then calling .save() appears to do nothing, and the python object has no allowed_to_push attribute to modify.

It does work for the basic .allow_force_push protectedbranch attribute, as added and tested in the original Pull Request.

Specifications

JohnVillalovos commented 6 months ago

I think historically we have left issues like this up to the user to handle. As I think the idea of python-gitlab is to be a thin wrapper over the GitLab API. The positive to doing this is it makes things easier for the users. The negative is that it adds more work for the volunteer developers of python-gitlab to add this and there are other inconsistencies in the upstream API that would thus wanted to be added. And then the maintenance of this if GitLab changes the API.

This might be a good thing to raise upstream and ask GitLab to resolve this inconsistency.

Interested to get @nejch opinion about this.

@nickbroon Off topic, I was in Edinburgh just two days ago 😄 But now back in the Portland, Oregon area.

github-actions[bot] commented 3 weeks ago

This issue was marked stale because it has been open 60 days with no activity. Please remove the stale label or comment on this issue. Otherwise, it will be closed in 15 days.

nickbroon commented 3 weeks ago

The only work around I can think to allow modifying branch permissions is for the user to GET protectedBranch, implement their own translation code from push_access_levels structure to the allowed_to_push and then POST that.

nejch commented 3 weeks ago

The only work around I can think to allow modifying branch permissions is for the user to GET protectedBranch, implement their own translation code from push_access_levels structure to the allowed_to_push and then POST that.

@nickbroon this is what I do, yes, I call an update() on the manager itself, when encountering this.

I'm honestly just afraid we'd deviate from upstream parameters if we were making aliases/mappings between GET and PUT/POST/PATCH endpoints, and then confuse users even more. You wouldn't really be updating push_access_levels if we added a mapping, for example.

I'm assuming we'd be doing this in a generic way and not for every possible endpoint, similar to how we have _create_attrs, _update_attrs etc, but as a dict for the mapping.