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

useradd.py: Use contextmanager to prevent leaked filehandles #27035

Merged
merged 2 commits into from Sep 11, 2015

Conversation

terminalmage
Copy link
Contributor

This also makes some optimizations by not assigning the output of
functions to variables when unnecessary, and changes this module to pass
arguments to the cmd.run functions as list instead of a string.

It also changes some weird behavior for the loginclass functions.
There's no reason to return a dictionary that will only have one
element, and there is no reason to return a string that is literally
just two quotes (i.e. '""') when there is no login class found. An empty
string is sufficient, and has the benefit of also evaluating as False as
a boolean.

This also makes some optimizations by not assigning the output of
functions to variables when unnecessary, and changes this module to pass
arguments to the cmd.run functions as list instead of a string.

It also changes some weird behavior for the loginclass functions.
There's no reason to return a dictionary that will only have one
element, and there is no reason to return a string that is literally
just two quotes (i.e. '""') when there is no login class found. An empty
string is sufficient, and has the benefit of also evaluating as False as
a boolean.
s0undt3ch added a commit that referenced this pull request Sep 11, 2015
useradd.py: Use contextmanager to prevent leaked filehandles
@s0undt3ch s0undt3ch merged commit b9baa0b into saltstack:2015.5 Sep 11, 2015
@terminalmage terminalmage deleted the useradd-contextmanager branch November 30, 2015 21:07
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.

None yet

2 participants