HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

ConfigParser - class for configuration managment

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
configparserformanagmentconfigurationclass

Problem

I writing tool to manage our application.

One part of it - is INI-file, which contains UUIDs for some application modules.

It looks like:

[version_1_0_2_staging]
MATH_DLL = 7a00ca94-b68d-4c89-a004-d3a4f5e7cf56
MATH_DLL_XML = 738174f7-f310-4989-8cae-da1690487e7c
ASSETMANAGER_API_DLL = 2e9e3807-4f8a-4ae1-a873-4a8f6d152eaf
ASSETMANAGER_API_DLL_XML = 024130cf-cbd9-41c2-b432-db9bc8565220
...
[debug]
MATH_DLL = 7b73c8f2-c5b6-435d-b310-37628302d1a0
MATH_DLL_XML = 5a349f50-76c3-456e-a885-f893c501a050
NETWORK_IMPL_DLL = 90a7b774-aa9a-4b85-b5e7-822333860588
...


And so on.

Script have few parts. Main is RDSmanager.py, where options defined:

parser_unity = subparsers.add_parser('unity', help='Unity application')
parser_unity.set_defaults(func=handler_unity)
...
    # UUIDs management
parser_unity.add_argument('-u', '--uuidgen', action='store', dest='unity_uuidgen',
                          help='Generate UUIDs for each module in Plugins.sln. Must be called with \'config_name\' as first argument and optionally - with `-b ci` option')
parser_unity.add_argument('-l', '--configlist', action='store_true', dest='unity_configlist',
                          help='Display configuration names in uuids.ini')
parser_unity.add_argument('-x', '--remove', action='store', dest='unity_uuid_remove',
                          help='Remove configuration from uuids.ini. Must be called with config_name to delete')
...


Than - function to parse options:

```
def handler_unity(action_list):
...
if action_list.unity_uuidgen:
logger.logger.info('Running UUID generator with %s' % action_list.unity_uuidgen)
from lib.unity.uuidgen import UuuidManagment
u = UuuidManagment(logger, RDS_BASEDIR)
u.uuidgen(action_list.unity_uuidgen, action_list.unity_buildtype)

if action_list.unity_configlist:
logger.logger.info('Running configlist')
from lib.unity.uuidgen import UuuidManagment
u = UuuidManagment(logger, RDS_BASEDIR)

Solution

PEP8

This is a style guide for Python that says how code should look.

  • Keep lines to a maximum 79 characters.



Comments are the only exception at 72.

-
Use snake_case for everything.

There are a few exceptions. The main one being classes which are CamelCase.

Just so you know, you are allowed to us full caps acronyms. UUIDManagment is a fine name.

  • Try to limit the amount of empty lines.


However.

  • Module level functions and classes should have two blank lines above and below them.



  • Methods should have one blank line above and below them.



Your style is really good otherwise.

PEP257

This is to do with doc-strings.

It doesn't matter wether you have the words on the apostrophes.

"""A multi-line
docstring.
"""

"""
A multi-line
docstring.
"""


Both act in the same way. Personally I think the latter is better, but each to there own.

I would discourage indenting the docstring so that it is not in-line with the apostrophes.

Other than that, they seem good.

Code

'%s' % var is deprecated. Use str.format instead.
There are bonuses to using the new format.
And less bugs, and edge cases.

raise NoConfigError(self.logger, 'No config file %s found' % self.config_file)
# To
raise NoConfigError(self.logger, 'No config file {} found'.format(self.config_file))


For an example of str.format

>>> '{}, {}, {}'.format('a', 'b', 'c')  # 2.7+ only
'a, b, c'
>>> '{2}, {1}, {0}'.format('a', 'b', 'c')
'c, b, a'
>>> 'Coordinates: {latitude}, {longitude}'.format(latitude='37.24N',
                                                  longitude='-115.81W')
'Coordinates: 37.24N, -115.81W'
>>> "repr() shows quotes: {!r}; str() doesn't: {!s}".format('test1', 'test2')
"repr() shows quotes: 'test1'; str() doesn't: test2"

>>> "int: {0:d};  hex: {0:x};  oct: {0:o};  bin: {0:b}".format(42)
'int: 42;  hex: 2a;  oct: 52;  bin: 101010'


It's quite remarkable, and somehow I learn something new every time I go on the page.

It's normally better to allow Python to handle exits.

except WindowsError as error:
    self.logger.logger.info('ERROR: %s' % error)
    sys.exit(1)


You could at a later-date find out a way to fix this error, or a work around for it. However you will need to remove the sys.exit to do either. It may be better to just raise the error again.

Also it is normally better to have small try statements. If for some reason self.logger raised a WindowsError, you may not handle things the same way.

You use both if mod_tag: and if 'CLOUD' in mod_tag: without their elif or else counterparts.

You can combine them to give you if mod_tag and 'CLOUD' in mod_tag.

In most cases this will be enough.
However if mod_tag returns True if there is data in mod_tag.
And if 'CLOUD' in mod_tag is a subset of this as 'CLOUD' has to be in the data.
And so mod_tag will be true every time 'CLOUD' in mod_tag is.

Also, try to reduce the size of the try statement. If isdev(build_type) raised AttributeError, would you handle this the same way, for example.

mod_tag = parser(self.logger, modname, mod_proj_file)
if 'CLOUD' in mod_tag:
    if isdev(build_type):
        try:
            self.config.set(config_name, modname.upper() + '_DLL', modname + '.dll')
            self.config.set(config_name, modname.upper() + '_DLL_XML', modname + '.dll.xml')
        except AttributeError:
            pass
    else:
        try: 
            self.config.set(config_name, modname.upper() + '_DLL', str(uuid.uuid4()))
            self.config.set(config_name, modname.upper() + '_DLL_XML', str(uuid.uuid4()))
        except AttributeError:
            pass


for name in additional.keys():
    if not isdev(build_type):
        self.config.set(config_name, name, str(uuid.uuid4()))
    else:
         for mod_var, mod_name in additional.items():
             self.config.set(config_name, mod_var, mod_name)


Try to not have nested for loops, unless it is really needed. This can be written to the below, as build_type does not change.

if not isdev(build_type):
    for name in aditional:
        self.config.set(config_name, name, str(uuid.uuid4()))
else:
    for mod_var, mod_name in additional.items():
        self.config.set(config_name, mod_var, mod_name)


Try to not use variables such as i. It's 'ok' when the program is small, however it is not descriptive, and when programs become larger they are a pain.

Code Snippets

"""A multi-line
docstring.
"""

"""
A multi-line
docstring.
"""
raise NoConfigError(self.logger, 'No config file %s found' % self.config_file)
# To
raise NoConfigError(self.logger, 'No config file {} found'.format(self.config_file))
>>> '{}, {}, {}'.format('a', 'b', 'c')  # 2.7+ only
'a, b, c'
>>> '{2}, {1}, {0}'.format('a', 'b', 'c')
'c, b, a'
>>> 'Coordinates: {latitude}, {longitude}'.format(latitude='37.24N',
                                                  longitude='-115.81W')
'Coordinates: 37.24N, -115.81W'
>>> "repr() shows quotes: {!r}; str() doesn't: {!s}".format('test1', 'test2')
"repr() shows quotes: 'test1'; str() doesn't: test2"

>>> "int: {0:d};  hex: {0:x};  oct: {0:o};  bin: {0:b}".format(42)
'int: 42;  hex: 2a;  oct: 52;  bin: 101010'
except WindowsError as error:
    self.logger.logger.info('ERROR: %s' % error)
    sys.exit(1)
mod_tag = parser(self.logger, modname, mod_proj_file)
if 'CLOUD' in mod_tag:
    if isdev(build_type):
        try:
            self.config.set(config_name, modname.upper() + '_DLL', modname + '.dll')
            self.config.set(config_name, modname.upper() + '_DLL_XML', modname + '.dll.xml')
        except AttributeError:
            pass
    else:
        try: 
            self.config.set(config_name, modname.upper() + '_DLL', str(uuid.uuid4()))
            self.config.set(config_name, modname.upper() + '_DLL_XML', str(uuid.uuid4()))
        except AttributeError:
            pass

Context

StackExchange Code Review Q#96453, answer score: 4

Revisions (0)

No revisions yet.