Skip to content

Fix all kwarg behaviour #701

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

Merged
merged 1 commit into from
Feb 20, 2019
Merged

Conversation

jpiron
Copy link

@jpiron jpiron commented Feb 18, 2019

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
Copy link
Member

max-wittig commented Feb 18, 2019

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

@max-wittig
Copy link
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
Contributor

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

@max-wittig
Copy link
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