Skip to content
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

gitlab_group => Make most options optional #6712

Merged
merged 9 commits into from
Jun 18, 2023

Conversation

Intellium
Copy link
Contributor

@Intellium Intellium commented Jun 15, 2023

SUMMARY

This PR changes the gitlab_group module to make most options optional, as they should be according to the gitlab API documentation. This PR also ensures compatibility with Gitlab v16 as current module behaviour is non-functional with Gitlab v16.

Closes #4577
Closes #5806

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

gitlab_group

ADDITIONAL INFORMATION

Current module behaviour is to include parameters in the API call to gitlab even if they're not set. This results in the API call including 'nil' values for these parameters. This still worked up until Gitlab v15.11.4.
Gitlab v16 currently breaks on this behaviour. This pull request ensures API parameters are only included when they're actually set, or if they are mandatory.

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added bug This issue/PR relates to a bug module module new_contributor Help guide this first time contributor plugins plugin (any type) labels Jun 15, 2023
Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution! Can you please add a changelog fragment? Thanks.

Also, instead of having an if for every entry, why not modify create_group() to filter out all None values?

plugins/modules/gitlab_group.py Outdated Show resolved Hide resolved
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-6 backport-7 Automatically create a backport for the stable-7 branch labels Jun 15, 2023
@nejch
Copy link
Contributor

nejch commented Jun 15, 2023

+1 for checking None values as there are a lot of other boolean values that could be added from https://docs.gitlab.com/ee/api/groups.html#new-group.

Btw, this closes #4577 and #5806.

(but as we discussed in #4577, since GitLab doesn't like receiving null attributes there will be another 15+ attributes from that API that would need to be added the same way eventually)

@Intellium
Copy link
Contributor Author

I added a filter to the create_group function instead.

@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jun 15, 2023
@ansibullbot ansibullbot removed ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jun 15, 2023
@Intellium
Copy link
Contributor Author

@felixfontein @nejch : Are these changed good ?

@ansibullbot

This comment was marked as outdated.

@ansibullbot ansibullbot added ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jun 15, 2023
@ansibullbot ansibullbot removed ci_verified Push fixes to PR branch to re-run CI needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jun 15, 2023
Intellium and others added 2 commits June 16, 2023 09:27
…s.yml

Co-authored-by: Felix Fontein <felix@fontein.de>
Co-authored-by: Felix Fontein <felix@fontein.de>
@Intellium
Copy link
Contributor Author

Committed the suggestions. Thanks @felixfontein !

Copy link
Contributor

@nejch nejch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Jun 18, 2023
@felixfontein felixfontein merged commit e85b008 into ansible-collections:main Jun 18, 2023
135 of 136 checks passed
@patchback
Copy link

patchback bot commented Jun 18, 2023

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/e85b0080360100d34d0bd366b85a321743314440/pr-6712

Backported as #6724

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Jun 18, 2023
* Make most options optional as they should be

* Add filter to create_group instead

* Remove whitespace

* Add changelog fragment

* Added description and extension to fragment

* Update changelogs/fragments/6712-gitlab_group-filtered-for-none-values.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/gitlab_group.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Make Python 2.6 compatible.

* Another shot at compatibility.

---------

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit e85b008)
@patchback
Copy link

patchback bot commented Jun 18, 2023

Backport to stable-7: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-7/e85b0080360100d34d0bd366b85a321743314440/pr-6712

Backported as #6725

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@felixfontein
Copy link
Collaborator

@Intellium thanks for your contribution!
@nejch thanks for reviewing!

patchback bot pushed a commit that referenced this pull request Jun 18, 2023
* Make most options optional as they should be

* Add filter to create_group instead

* Remove whitespace

* Add changelog fragment

* Added description and extension to fragment

* Update changelogs/fragments/6712-gitlab_group-filtered-for-none-values.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/gitlab_group.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Make Python 2.6 compatible.

* Another shot at compatibility.

---------

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit e85b008)
felixfontein pushed a commit that referenced this pull request Jun 18, 2023
…ons optional (#6725)

gitlab_group => Make most options optional (#6712)

* Make most options optional as they should be

* Add filter to create_group instead

* Remove whitespace

* Add changelog fragment

* Added description and extension to fragment

* Update changelogs/fragments/6712-gitlab_group-filtered-for-none-values.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/gitlab_group.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Make Python 2.6 compatible.

* Another shot at compatibility.

---------

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit e85b008)

Co-authored-by: Intellium <w.moeken@moeken.eu>
felixfontein pushed a commit that referenced this pull request Jun 18, 2023
…ons optional (#6724)

gitlab_group => Make most options optional (#6712)

* Make most options optional as they should be

* Add filter to create_group instead

* Remove whitespace

* Add changelog fragment

* Added description and extension to fragment

* Update changelogs/fragments/6712-gitlab_group-filtered-for-none-values.yml

Co-authored-by: Felix Fontein <felix@fontein.de>

* Update plugins/modules/gitlab_group.py

Co-authored-by: Felix Fontein <felix@fontein.de>

* Make Python 2.6 compatible.

* Another shot at compatibility.

---------

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit e85b008)

Co-authored-by: Intellium <w.moeken@moeken.eu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-7 Automatically create a backport for the stable-7 branch bug This issue/PR relates to a bug module module new_contributor Help guide this first time contributor plugins plugin (any type)
Projects
None yet
4 participants