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

rabbitmq_user: Specify key to use while sorting permissions #49126

Merged
merged 1 commit into from Nov 30, 2018
Merged

rabbitmq_user: Specify key to use while sorting permissions #49126

merged 1 commit into from Nov 30, 2018

Conversation

groggi
Copy link
Contributor

@groggi groggi commented Nov 26, 2018

SUMMARY

Without specifying the dictionary key to use while sorting it will fail
in Python 3 environments due to simplifying Python's rules for ordering
comparisons: https://docs.python.org/3.0/whatsnew/3.0.html#ordering-comparisons

Fixes #49120

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

rabbitmq_user

ADDITIONAL INFORMATION

The basic idea is to use the vhost dictionary key for sorting, which should be unique in the list according to https://www.rabbitmq.com/access-control.html#permissions ("Permissions are expressed as a triple of regular expressions - one each for configure, write and read - on per-vhost basis.").

I didn't find a unit test file I could extend, so no tests added. Please let me know if I just didn't look hard enough.

My Python is a bit rusty, so please let me know if I can improve it or feel free to base a new patch for #49120 on it.

Without specifying the dictionary key to use while sorting it will fail
in Python 3 environments due to simplifying Python's rules for ordering
comparisons: https://docs.python.org/3.0/whatsnew/3.0.html#ordering-comparisons
@ansibot
Copy link
Contributor

ansibot commented Nov 26, 2018

Hi @groggi, thank you for submitting this pull-request!

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Nov 26, 2018

@ansibot ansibot added affects_2.8 This issue/PR affects Ansible v2.8 bug This issue/PR relates to a bug. community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. python3 small_patch support:community This issue/PR relates to code supported by the Ansible community. labels Nov 26, 2018
@gundalow gundalow changed the title Specify key to use while sorting permissions rabbitmq_user: Specify key to use while sorting permissions Nov 26, 2018
@gundalow gundalow removed needs_triage Needs a first human triage before being processed. labels Nov 28, 2018
@gundalow
Copy link
Contributor

@windowsrefund Could you please test this patch?

@groggi Given your comment #49120 (comment) does this need further work/testing?

@groggi
Copy link
Contributor Author

groggi commented Nov 29, 2018

@groggi Given your comment #49120 (comment) does this need further work/testing?

This patch does not cover the issue of the module being idempotent. It only resolves the original Python exception mentioned in #49120.

I'm not sure how quickly I'll get to identifying and resolving the idempotency issue. It might make sense to resolve #49120 with this patch and open an independent issue to track the idempotency of the module (as was identified by @windowsrefund #49120 (comment)). Let me know if I should create another bug report or if we should track it as part of #49120.

@gundalow
Copy link
Contributor

@groggi Given the above, I'll merge this as it's an improvement over what we have today.

Whatever works for you is fine :)

@gundalow gundalow merged commit 4096d74 into ansible:devel Nov 30, 2018
@gundalow
Copy link
Contributor

Merged into devel for release in Ansible 2.8
If you'd like this fix in Ansible 2.7 please raise a backport PR with changelog

https://docs.ansible.com/ansible/devel/community/development_process.html#backport-pull-request-process

@gundalow
Copy link
Contributor

@groggi oh, your first pull request, MERGED, congratulations!

@groggi
Copy link
Contributor Author

groggi commented Nov 30, 2018

@gundalow thank you a lot!

@gundalow
Copy link
Contributor

gundalow commented Dec 4, 2018

@groggi If you search for rabbitmq on https://ansible.sivel.net/pr/byfile.html looks like there are a number of open PRs in progress. I'm sure they could benefit for your knowledge :)

@gundalow gundalow added the suggestion_given ContribEx: Idea for next work given label Dec 4, 2018
kbreit pushed a commit to kbreit/ansible that referenced this pull request Jan 11, 2019
Without specifying the dictionary key to use while sorting it will fail
in Python 3 environments due to simplifying Python's rules for ordering
comparisons: https://docs.python.org/3.0/whatsnew/3.0.html#ordering-comparisons
@dagwieers dagwieers added the rabbitmq RabbitMQ community label Jan 28, 2019
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.8 This issue/PR affects Ansible v2.8 bug This issue/PR relates to a bug. community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. new_contributor This PR is the first contribution by a new community member. python3 rabbitmq RabbitMQ community small_patch suggestion_given ContribEx: Idea for next work given support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RabbitMQ user configuration fails due to sorting list of dicts
5 participants