Skip to content

Commit

Permalink
Don't pass mutables as defaults
Browse files Browse the repository at this point in the history
  • Loading branch information
thatch45 committed Jan 3, 2014
1 parent 7c7c9aa commit f8cc172
Showing 1 changed file with 15 additions and 6 deletions.
21 changes: 15 additions & 6 deletions salt/states/ini_manage.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def __virtual__():
return 'ini' if 'ini_manage.set_option' in __salt__ else False

This comment has been minimized.

Copy link
@Akilesh1597

Akilesh1597 Jan 10, 2014

Contributor

Shouldn't this be return 'ini' if 'ini.set_option' in salt else False



def options_present(name, sections={}):
def options_present(name, sections=None):
'''
/home/saltminion/api-paste.ini:
ini_manage:
Expand All @@ -35,6 +35,8 @@ def options_present(name, sections={}):
dict will be untouched
changes dict will contain the list of changes made
'''
if sections is None:
sections = {}

This comment has been minimized.

Copy link
@SEJeff

SEJeff Jan 6, 2014

Contributor

Can be shortened using the or operator ie:

sections = sections or {}
ret = {'name': name,
'changes': {},
'result': True,
Expand Down Expand Up @@ -65,7 +67,7 @@ def options_present(name, sections={}):
return ret


def options_absent(name, sections={}):
def options_absent(name, sections=None):
'''
/home/saltminion/api-paste.ini:
ini_manage:
Expand All @@ -81,6 +83,8 @@ def options_absent(name, sections={}):
dict will be untouched
changes dict will contain the list of changes made
'''
if sections is None:
sections = {}
ret = {'name': name,
'changes': {},
'result': True,
Expand All @@ -107,7 +111,7 @@ def options_absent(name, sections={}):
return ret


def sections_present(name, sections={}):
def sections_present(name, sections=None):
'''
/home/saltminion/api-paste.ini:
ini_manage:
Expand All @@ -122,6 +126,8 @@ def sections_present(name, sections={}):
options present in file and not specified in sections will be deleted
changes dict will contain the sections that changed
'''
if sections is None:
sections = {}
ret = {'name': name,
'changes': {},
'result': True,
Expand Down Expand Up @@ -151,7 +157,7 @@ def sections_present(name, sections={}):
return ret


def sections_absent(name, sections=[]):
def sections_absent(name, sections=None):
'''
/home/saltminion/api-paste.ini:
ini_manage:
Expand All @@ -163,15 +169,18 @@ def sections_absent(name, sections=[]):
options present in file and not specified in sections will be deleted
changes dict will contain the sections that changed
'''
if sections is None:
sections = []
ret = {'name': name,
'changes': {},
'result': True,
'comment': 'No anomaly detected'
}
if __opts__['test']:
ret['result'] = None
ret['comment'] = 'ini file {0} shall be validated for absense of\
given sections'.format(name)
ret['comment'] = ('ini file {0} shall be validated for absence of '
'given options under their respective '
'sections').format(name)
return ret
for section in sections:
cur_section = __salt__['ini.remove_section'](name, section)
Expand Down

3 comments on commit f8cc172

@SEJeff
Copy link
Contributor

@SEJeff SEJeff commented on f8cc172 Jan 6, 2014

Choose a reason for hiding this comment

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

Great commit however. Doing this prevents soooo many hard to troubleshoot bugs.

@thatch45
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good catch on making this more terse. And this is why we have lint checks for these sort of things :)

@Akilesh1597
Copy link
Contributor

Choose a reason for hiding this comment

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

There are some minor changes to make. I have highlighted them at 31cd3e6

Please sign in to comment.