Repository navigation
Filtering commits by ref_name does not work #566
Description
Description of the problem, including code/CLI snippet
When I try to get all commits for a project and filter them by branch, I still get all commits for all branches. E.g.:
commits = project.commits.list(all=True, ref_name='master')
As far as I understand the all=True only concerns pagination and should not influence the ref_name filter, and the master branch as ref_name should be a valid input (provided that this branch exists), right?
Expected Behavior
len(commits) should be 38 and only contains commits that are on the master branch.
This was also verified using the gitlab commits overview and checking if all returned commits are indeed on master.
Actual Behavior
len(commits) is 62 and also includes commits that are on other branches that were not merged to master, or that are on branches that have been deleted already.
Specifications
- python-gitlab version: 1.5.1
- API version you are using (v3/v4): v4
- Gitlab server version (or gitlab.com): GitLab Community Edition 11.0.4 edb037c
Reacted by Gerard Capes
Activity
gpocentek commentedon Aug 24, 2018
Hi @AlexanderLill
Thanks for reporting the problem.
I have been able to reproduce this bug. It's a mixup between the
allattributes in python-gitlab and the Gitlab API.gpocentek commentedon Aug 25, 2018
This is in fact a nasty design bug. I don't think it can be easily fixed without changes to the python-gitlab API.
But you can achieve the desired result using the
as_list=Falseparameter instead ofall=True. Thelist()method will return a generator instead of a list, but you can easily create a list from there (which is whatall=Truedoes internally anyway):Let me know if this helps.
AlexanderLill commentedon Sep 2, 2018
Hi @gpocentek ,
thanks for the workaround, it works!
I would be interested in understanding the issue better, as I currently can't grasp the root cause for this issue.
Also I've been wondering, how did you come up with the overall architecture of the
python-gitlabpackage? Is this a common solution for implementing clients for a REST API? I was delighted seeing the structure and simplicity of the managers being assigned to the object classes and so on, and I've seen many more complicated implementations of APIs so far.gpocentek commentedon Sep 5, 2018
Hi @AlexanderLill
The
allparameter has 2 meanings:The problem is that python-gitlab has no way of knowing which meaning is the correct one, and sends the
allparameter to the GitLab server, resulting in the behaviour that you didn't want for this specific API call (the parameter is simply ignored by the server for other API calls). Bad design decision!By design python-gitlab accepts anything that the user wants to send to the server, and trusts the server to return information about bad or missing parameters. So there are no pre-checks or validation on the client side. I made this choice because the GitLab API evolves very quickly, and tracking and implementing all the changes is a big maintenance burden. The other benefit is that python-gitlab is not tied to a specific GitLab server version, and I don't have to manage dozens of releases.
This choice and the manager pattern that has been implement for the v4 API are inspired by the OpenStack clients (https://github.com/openstack/python-*client). The manager pattern is really useful to manage objects creation and manipulation, and really matches the REST API design. I'm not sure at all if this is a common architecture for API clients, but I think it works quite well.
I hope this answers your questions :)
My idea to fix this bug is to simply drop the
allattribute (with a deprecation release before removal) and always return the generator instead of a list.fix(api): avoid parameter conflicts with python and gitlab
fix(api): avoid parameter conflicts with python and gitlab
fix(api): avoid parameter conflicts with python and gitlab