Skip to content

Fix all kwarg behaviour - #701

Merged
gpocentek merged 1 commit into
python-gitlab:masterfrom
jpiron:fix_all_behaviour
Feb 20, 2019
Merged

Fix all kwarg behaviour#701
gpocentek merged 1 commit into
python-gitlab:masterfrom
jpiron:fix_all_behaviour

Conversation

@jpiron

@jpiron jpiron commented Feb 18, 2019

Copy link
Copy Markdown

all kwarg is used to manage GitlabList generator behaviour.
However, as it is not pop'ed from kwargs, it is sent to Gitlab API.
Some endpoints such as the project commits one,
support a all attribute.
This means a call like project.commits.list(all=True, ref_name='master')
won't return all the master commits as one might expect but all the
repository's commits.
To prevent confusion, the same kwarg shouldn't be used for 2 distinct
purposes.
Moreover according to the documentation,
the all project commits API endpoint attribute doesn't seem supported.

`all` kwarg is used to manage GitlabList generator behaviour.
However, as it is not poped from kwargs, it is sent to Gitlab API.
Some endpoints such as [the project commits](https://docs.gitlab.com/ee/api/commits.html#list-repository-commits) one,
support a `all` attribute.
This means a call like `project.commits.list(all=True, ref_name='master')`
won't return all the master commits as one might expect but all the
repository's commits.
To prevent confusion, the same kwarg shouldn't be used for 2 distinct
purposes.
Moreover according to [the documentation](https://python-gitlab.readthedocs.io/en/stable/gl_objects/commits.html#examples),
the `all` project commits API endpoint attribute doesn't seem supported.
@max-wittig

max-wittig commented Feb 18, 2019

Copy link
Copy Markdown
Member

Makes sense. To use the all parameter, you can supply it with query_parameters instead (starting with 1.8)

Comment thread gitlab/__init__.py
@max-wittig

Copy link
Copy Markdown
Member

@gpocentek We may need to update the documentation that all is not send to the GitLab API anymore by default. What do you think?

@gpocentek

Copy link
Copy Markdown
Contributor

@max-wittig yes, we need to add that info in the release notes

@max-wittig

Copy link
Copy Markdown
Member

@gpocentek Could you disable the travis requirement temporary to merge this?

@gpocentek
gpocentek merged commit 8ce4e9e into python-gitlab:master Feb 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants