3
\$\begingroup\$

I'm preparing a new tool for our deployment system.

One part of it is to generate a set of XML-files for modules.

The tool is really big, so I'll post only one function here for your consideration.

Prerequisites

We have a template for new XML files, which looks like this:

<plugin assemblyUrl="%DATAURLPREFIX%/%MOD_DLL%" debugInfoUrl="%DATAURLPREFIX%/%MOD_DLL%.mdb" id="%MOD_ID%">
  <dependencies>
  </dependencies>
</plugin>

Variables are enclosed in %% for historic reasons, so I just leave them as they are.

Next - we have a config file, which contains modules names (in view on %MODULE_NAME_DLL%), their XML file names (in view of MODULES_NAME_DLL_XML%) and their UUIDs:

[debug]
MATH_DLL = 98279b0a-7ec1-4b0f-8ab3-14f79a8526a4
MATH_DLL_XML = 75de8971-fae9-4932-9025-2730be983ec1
NETWORK_IMPL_DLL = dbd98894-2868-4a27-8a7b-688d53d5923e
NETWORK_IMPL_DLL_XML = f8d93ccf-a14c-4e51-86e9-e479b2ecb969
ASSETMANAGER_API_DLL = 42906e06-3df7-4430-82e6-9ea3ac25d1cf
...

Dependencies tale from each module's .csproj file:

  <PropertyGroup>
    <ModType>CLOUD</ModType>
  </PropertyGroup>
  <PropertyGroup>
    <DependencyId>MetaDataParser</DependencyId>
  </PropertyGroup>

There are also some other functions used, please let me know if I need to add them here too.

The function is called from the main file - RDSmanager.py, with:

parser.add_argument('-l', '--configlist', action='store_true', help=('Display configuration names in uuids.ini'))
...
if action_list.confgen:
    print('Running confgen')
    from lib.build import confgen
    confgen.main(RDS_BASEDIR, action_list.confgen, action_list.buildtype)
...

Finally, the function:

def xmlgen(rds_basedir, config_name, build_type):

    """Generate files which will be deployed to Cloudlibrary.

       rds_basedir - directory, where RDSmanager placed;
       config_name - section name in uuids.ini, from where take UUIDs;
       build_type - `dev` for developers, or `ci` for build server;

       Takes module's templates XML-files from Plugins\Package
       and save in to RDSmanager\conf\build\_xmls\`config_name`

       Rewrites variables in template file with data from uuids.ini"""

    # workdir - where source files placed
    workdir = checkhome()

    # used for substitution %DATAURLPREFIX% in module's XML-files
    url = geturl()

    # plugins name from Plugins.sln
    plugins = plugins_list(workdir)

    # where to save generated XML-files, e.g. D:\RDS\rdsmanager\conf\build\_xmls\debug
    outputdir = outdir(rds_basedir, config_name)

    # main UUID's list and conigs
    uuids_list = os.path.join(rds_basedir, 'conf', 'build', 'uuids.ini')

    # config object to work with uuids.ini and modules list woth mod's variables and UUIDs
    config, res = mods_uuids(uuids_list, config_name)

    if not os.path.isdir(outputdir):
        print('\n%s not found, will create...' % outputdir)
        try:
            os.mkdir(outputdir)
            print('Done.\n')
        except IOError as e:
            print('ERROR: %s' % e)
            sys.exit(1)

    for plugin in plugins:

        if plugin.startswith('Proj'):

            # just name like 'Math'
            modename = modes(plugin)
            # mod's .scproj file
            # e.g. D:\RDS\my_branches\setevoy_confgen\octopus-plugins\Plugins\Math\Math.csproj
            mod_proj_file = mod_proj(workdir, modename)

            # template for new generated XML files
            # dev have debugInfoUrl=%URL%
            if isdev(build_type):
                mode_xml_in = os.path.join(rds_basedir, 'conf', 'build', 'templates', 'template_dev.dll.xml')
            else:
                mode_xml_in = os.path.join(rds_basedir, 'conf', 'build', 'templates', 'template_ci.dll.xml')

            try:
                # tag like 'CLOUD', 'MAIN' etc
                mod_tag = parser(modename, mod_proj_file)

                if mod_tag:
                    if 'CLOUD' in mod_tag:

                        # where to write generated data
                        # if build_type = 'ci'
                        # e.g. D:\RDS\rdsmanager\conf\build\_xmls\debug\5472d50f-3b9c-4ec4-af4b-f8f2ece9d75d
                        # or if build_type = 'dev'
                        # D:\RDS\rdsmanager\conf\build\_xmls\develop\Math.dll.xml
                        mode_xml_out = os.path.join(outputdir, config.get(config_name, modename.upper() + '_DLL_XML'))

                        dependency_str = '    <dependency pid="%s" />\n'
                        # lis of dependecies
                        # e.g. MetaDataParser, Test
                        dependency = dependency_finder(mod_proj_file)

                        if os.path.isfile(mode_xml_in):
                            print('Module: %s XML file found: %s\n' % (modename, mode_xml_in))

                            # load template
                            with open(mode_xml_in, 'r') as inxml:
                                lines = inxml.readlines()

                                # prepare list to add lines from template
                                outdata = []

                                for line in lines:

                                    # change to 'file:///c:/cloudlibrary/data/' if isdev()
                                    # or url for example 'https://www.dev.reddotsquare.com/data' if not isdev()
                                    if '%DATAURLPREFIX%' in line:
                                        line = re.sub(r'%DATAURLPREFIX%', url + 'data', line)

                                    # replace with 'Math.dll' if isdev()
                                    # or 'ddf03268-e2d8-4ee4-bc6a-920818f24dc6' if not isdev()
                                    if '%MOD_DLL%' in line:
                                        line = re.sub(r'%MOD_DLL%', config.get(config_name, modename.upper() + '_DLL'), line)

                                    # repalce with modulename, like 'Math'
                                    if '%MOD_ID%' in line:
                                        line = re.sub(r'%MOD_ID%', modename, line)

                                    # start dependency resolution
                                    if line.startswith('  <dependencies>'):
                                        outdata.append('  <dependencies>\n')
                                        # if any found in .scproj
                                        if dependency:
                                            for a in dependency.split(', '):
                                                # append to list
                                                outdata.append(dependency_str % a)
                                        # finish dependency resolution
                                        outdata.append('  </dependencies>\n')

                                    # add other lines, like <plugin>, </plugin> etc
                                    if not line.startswith('  <dependencies>'):
                                        if not line.startswith('  </dependencies>'):
                                            outdata.append(line)

                                for i in outdata:
                                    print(i.strip('\n'))


                            with open(mode_xml_out, 'w') as outxml:
                                for i in outdata:
                                    outxml.write(i)
                                print('\nXML for %s saved as %s\n' % (modename, mode_xml_out))

                        else:
                            print('XML file %s for module %s not found, skipping.' % (mode_xml_in, modename))

            except AttributeError:
                pass
\$\endgroup\$
  • \$\begingroup\$ That seems like a very long function, and particularly a very long try block! \$\endgroup\$ – jonrsharpe Jun 2 '15 at 14:33
1
\$\begingroup\$

That function is pretty huge.

  • The output directory creation is a bit harsh; maybe os.makedirs would be nicer here. Is the sys.exit really justified? In any case sys.exit('ERROR %s' % e) would be better since it prints to standard error instead and all of this also justifies a separate function:

    def ensure_directory_or_fail(directory):
        if not os.path.isdir(directory):
            print('\n%s not found, will create...' % directory)
            try:
                os.mkdir(directory)
                print('Done.\n')
            except IOError as e:
                sys.exit('ERROR: %s' % e)
    

    You can also take a look here for more info.

  • There is no control flow disturbing the for plugin in plugins loop, so I'd suggest moving that out into a separate function first. Then a return may be used to get rid of lots of indentation, e.g.:

    if not mod_tag or 'CLOUD' not in mod_tag:
        return
    

    instead of the nested ifs, or:

    if not os.path.isfile(mode_xml_in):
        print('XML file %s for module %s not found, skipping.' % (mode_xml_in, modename))
        return
    

    instead of the nested if the other way round. The nesting to read the mode_xml_in file is also not necessary.

  • I'd get rid of the "foo" in line checks, as the regex replacement will already check if that string is contained.
  • The try/except block is a concern, but I have absolutely no idea where it would be triggered from. So I'd really urge you to wrap the exact location where this might be raised from instead of wrapping the whole code block in order to make finding bugs much easier. If you can prevent the exception from being raised by checking the existence of the attribute beforehand that would be even better.
  • The actual replacement part with scanning for XML fragments looks iffy, but if it works reliably I won't say much against it.
  • Moving the variable interpolation part into a separate function again facilitates separation of concerns and testing. It would also make sense to create a loop for the string replacements, or use a dedicated library for these kind of variable interpolations:

    def replace_variables(modename, data_url_prefix, mod_dll, line):
        replacements = [
            # change to 'file:///c:/cloudlibrary/data/' if isdev()
            # or url for example 'https://www.dev.reddotsquare.com/data' if not isdev()
            (r'%DATAURLPREFIX%', data_url_prefix),
            # replace with 'Math.dll' if isdev()
            # or 'ddf03268-e2d8-4ee4-bc6a-920818f24dc6' if not isdev()
            (r'%MOD_DLL%', mod_dll),
            # replace with modulename, like 'Math'
            (r'%MOD_ID%', modename)
        ]
    
        for (name, value) in replacements:
            line = re.sub(name, value, line)
    
        return line
    

All in all:

def ensure_directory_or_fail(directory):
    if not os.path.isdir(directory):
        print('\n%s not found, will create...' % directory)
        try:
            os.mkdir(directory)
            print('Done.\n')
        except IOError as e:
            sys.exit('ERROR: %s' % e)


def replace_variables(modename, data_url_prefix, mod_dll, line):
    replacements = [
        # change to 'file:///c:/cloudlibrary/data/' if isdev()
        # or url for example 'https://www.dev.reddotsquare.com/data' if not isdev()
        (r'%DATAURLPREFIX%', data_url_prefix),
        # replace with 'Math.dll' if isdev()
        # or 'ddf03268-e2d8-4ee4-bc6a-920818f24dc6' if not isdev()
        (r'%MOD_DLL%', mod_dll),
        # replace with modulename, like 'Math'
        (r'%MOD_ID%', modename)
    ]

    for (name, value) in replacements:
        line = re.sub(name, value, line)

    return line


def process_line(modename, dependency_str, mod_dll, dependency,
                 data_url_prefix, outdata, line):
    line = replace_variables(modename, data_url_prefix, mod_dll, line)

    # start dependency resolution
    if line.startswith('  <dependencies>'):
        outdata.append('  <dependencies>\n')
        # if any found in .scproj
        if dependency:
            for a in dependency.split(', '):
                # append to list
                outdata.append('    <dependency pid="%s" />\n' % a)
        # finish dependency resolution
        outdata.append('  </dependencies>\n')

    # add other lines, like <plugin>, </plugin> etc
    elif not line.startswith('  </dependencies>'):
        outdata.append(line)


def process_plugin(config_name, plugin, url, build_type, workdir, rds_basedir,
                   config, outputdir):
    # just name like 'Math'
    modename = modes(plugin)

    # mod's .scproj file
    # e.g. D:\RDS\my_branches\setevoy_confgen\octopus-plugins\Plugins\Math\Math.csproj
    mod_proj_file = mod_proj(workdir, modename)

    # template for new generated XML files
    # dev have debugInfoUrl=%URL%
    template_name = 'template_%s.dll.xml' % 'dev' if isdev(build_type) else 'ci'
    mode_xml_in = os.path.join(rds_basedir, 'conf', 'build', 'templates', template_name)

    try:
        # tag like 'CLOUD', 'MAIN' etc
        mod_tag = parser(modename, mod_proj_file)

        if not mod_tag or 'CLOUD' not in mod_tag:
            return

        # where to write generated data
        # if build_type = 'ci'
        # e.g. D:\RDS\rdsmanager\conf\build\_xmls\debug\5472d50f-3b9c-4ec4-af4b-f8f2ece9d75d
        # or if build_type = 'dev'
        # D:\RDS\rdsmanager\conf\build\_xmls\develop\Math.dll.xml
        mode_xml_out = os.path.join(outputdir, config.get(config_name, modename.upper() + '_DLL_XML'))

        # list of dependencies
        # e.g. MetaDataParser, Test
        dependency = dependency_finder(mod_proj_file)

        if not os.path.isfile(mode_xml_in):
            print('XML file %s for module %s not found, skipping.' % (mode_xml_in, modename))
            return

        print('Module: %s XML file found: %s\n' % (modename, mode_xml_in))

        # load template
        with open(mode_xml_in, 'r') as inxml:
            lines = inxml.readlines()

        # prepare list to add lines from template
        outdata = []

        data_url_prefix = url + 'data'
        mod_dll = config.get(config_name, modename.upper() + '_DLL')

        for line in lines:
            process_line(modename, mod_dll, dependency, data_url_prefix,
                         outdata, line)

        for i in outdata:
            print(i.strip('\n'))

        with open(mode_xml_out, 'w') as outxml:
            for i in outdata:
                outxml.write(i)

        print('\nXML for %s saved as %s\n' % (modename, mode_xml_out))

    except AttributeError:
        pass


def xmlgen(rds_basedir, config_name, build_type):

    """Generate files which will be deployed to Cloudlibrary.

       rds_basedir - directory, where RDSmanager placed;
       config_name - section name in uuids.ini, from where take UUIDs;
       build_type - `dev` for developers, or `ci` for build server;

       Takes module's templates XML-files from Plugins\Package
       and save in to RDSmanager\conf\build\_xmls\`config_name`

       Rewrites variables in template file with data from uuids.ini"""

    # workdir - where source files placed
    workdir = checkhome()

    # used for substitution %DATAURLPREFIX% in module's XML-files
    url = geturl()

    # plugins name from Plugins.sln
    plugins = plugins_list(workdir)

    # where to save generated XML-files, e.g. D:\RDS\rdsmanager\conf\build\_xmls\debug
    outputdir = outdir(rds_basedir, config_name)

    # main UUID's list and conigs
    uuids_list = os.path.join(rds_basedir, 'conf', 'build', 'uuids.ini')

    # config object to work with uuids.ini and modules list woth mod's variables and UUIDs
    config, res = mods_uuids(uuids_list, config_name)

    ensure_directory_or_fail(outputdir)

    for plugin in plugins:
        if plugin.startswith('Proj'):
            process_plugin(config_name, plugin, url, build_type, workdir,
                           rds_basedir, config, outputdir)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.