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

Fix issue with duplicates in user.list_users on OSX #49376

Merged
merged 6 commits into from Aug 30, 2018

Conversation

Projects
None yet
6 participants
@twangboy
Copy link
Contributor

commented Aug 28, 2018

What does this PR do?

Fixes the issue where user.list_users was showing duplicate users names. Uses a set to do this as a set does not allow duplicates.

What issues does this PR fix or reference?

#48608

Tests written?

No

Commits signed with GPG?

Yes

@twangboy twangboy requested a review from saltstack/team-core Aug 28, 2018

@cachedout

This comment has been minimized.

Copy link
Collaborator

commented Aug 28, 2018

This does not strike me as the right fix. IMHO, we need to figure out why the duplication is happening to begin with.

@DmitryKuzmenko

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2018

At least it's interesting why

>>> for p in pwd.getpwall():
...     print p[0]

gets a list of non-dups, but [user.pw_name for user in pwd.getpwall()] returns duplications. o_O

@twangboy

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2018

@DmitryKuzmenko There are still dups. The list is not sorted, so it's hard to tell

@twangboy

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2018

@cachedout I think the issue is with the pwd.getpwall command on osx. I don't get dupes on linux boxes. Is there a problem with filtering them out?

@twangboy

This comment has been minimized.

Copy link
Contributor Author

commented Aug 28, 2018

@cachedout @DmitryKuzmenko Changed it to use dscl

@terminalmage
Copy link
Contributor

left a comment

I usually see this as /Users and not /users. If dscl can use case-insensitive names, then I think this is fine.

@twangboy

This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2018

@terminalmage Works fine with /users or /Users. Seems to be NOT case sensitive.

@cachedout

This comment has been minimized.

Copy link
Collaborator

commented Aug 29, 2018

@twangboy We have a failed test here I'm afraid.

twangboy added some commits Aug 29, 2018

@rallytime rallytime merged commit 7a166bc into saltstack:2017.7 Aug 30, 2018

3 of 10 checks passed

continuous-integration/jenkins/pr-merge This commit cannot be built
Details
jenkins/pr/py2-centos-7 The py2-centos-7 job has failed
Details
jenkins/pr/py2-ubuntu-1604 The py2-ubuntu-1604 job has failed
Details
jenkins/pr/py2-windows-2016 The py2-windows-2016 job has failed
Details
jenkins/pr/py3-centos-7 The py3-centos-7 job has failed
Details
jenkins/pr/py3-ubuntu-1604 The py3-ubuntu-1604 job has failed
Details
jenkins/pr/py3-windows-2016 The py3-windows-2016 job has failed
Details
WIP ready for review
Details
jenkins/pr/docs The docs job has passed
Details
jenkins/pr/lint The lint job has passed
Details

@twangboy twangboy deleted the twangboy:fix_48608 branch Aug 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.