current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite

Script designed to avoid manually edit configuration files by support.

Config file example:

$ cat ../secure/test.property
[Connection settings]
jdbc.main.url = jdbc:oracle:thin:@host1
jdbc.main.username = USER
jdbc.main.password = pass

[Mail settings]
mail.smtp.host=127.0.0.1
mail.smtp.port=25
mail.smtp.on=false
mail.cronExpression=0 0/2 * ? * MON-SUN

[Import limits cron expressions]
gateKeeper.cronExpression=0 0 5 ? * MON-SUN
cleanup.compress.cronExpression=0 0 12 ? * SUN
send.mails.to.group.cronExpression = 0 0 12 ? * MON-SUN
exc_tckt_d.cronExpression=0 30 22 ? * MON-FRI

[Gate keeper settings]
gk.homedir=gate_keeper_report
sftp.useLocalFileIfAvailable=true
sftp.downloadBaseDir=~/download
sftp.maxRetryLimit=3

[TCP Report Extract Properties]
usePrecalculatedAggregation=true

[Other settings]
bean.datasource.query_log_wrapper=mainDataSourceWrapper
log.client.exception = false
time.to.keep.domain=7*12
time.to.keep.uncompress=1
dao.batch.size.max=30

The script uses the ConfigParser module, to select and update values. As it is quite large in total, I'll place only a few main functions here.

...
ENV = os.environ['ENV']
CONF_DIR = os.path.join(os.environ['HOME'], 'secure')
LOG =  os.path.join(os.environ['HOME'], 'logs/settings.log')

logging.basicConfig(format = '%(filename)s[LINE:%(lineno)d] - %(levelname)-3s [%(asctime)s] %(message)s ', filename=LOG, level=logging.DEBUG)
...


def select_conf(path):

    '''Used to select configuration file from directory in CONF_DIR variable.
       Return filename without full path.'''

    while True:

        list = []

        for file in os.listdir(path):
            list.append(file)
            print('%s %s' % ((list.index(file)), file))

        try:
            res_file = int(raw_input('\nPlease, select file to edit: '))
            print('You selected: %s' % list[res_file])
        except ValueError as e:
            print('Please, enter correct value: %s\n' % e)
            sys.exit(2)

        if answer('Is it OK? [y/n] '):
            logging.info('Selected %s file to work with.' % list[res_file])
            return(list[res_file])
            break


def select_section(configfile):

    '''Use ConfigParser module to select sections, declared in [] blocks in file.
       File takes from select_conf() function.'''

    Config = ConfigParser.ConfigParser()
    file =  os.path.join(CONF_DIR, configfile)
    list = []

    try:
        Config.read(file)
    except ConfigParser.MissingSectionHeaderError as e:
        print(e)
        sys.exit(5)


    print('\n')

    while True:

        for section in Config.sections():
            list.append(section)
            print('%s %s' % ((list.index(section)), section))

        try:
            res_section = int(raw_input('\nPlease, select section to edit: '))
            print('You selected: %s' % list[res_section])
        except ValueError as e:
            print('Please, enter correct value: %s\n' % e)
            sys.exit(3)

        if answer('Is it OK? [y/n] '):
            logging.info('Selected %s section to work with.' % list[res_section])
            return(list[res_section], file)
            break


def edit_option():

    '''Use $section variable from select_section() function, to set section, to select options from,
       and $file variable from select_section() function, to set file, to be edited.

       First loop - to set new variable $res_option, where option to be edited saves;
       second loop - to set new varaible $new_value, to be saved in $section in $file.
       next. in second loop, with ConfigParcer - writing new $new_value in $file.'''

    section, file = select_section(select_conf(CONF_DIR))
    Config = ConfigParser.ConfigParser()

    list = []

    Config.read(file)

    print('\n')

    while True:

        for option in Config.options(section):
            list.append(option)
            print('%s %s' % ((list.index(option)), option))

        try:
            res_option = int(raw_input('\nPlease, select option to edit: '))
            print('You selected: %s' % list[res_option])
        except ValueError as e:
            print('Please, enter correct value: %s\n' % e)
            sys.exit(4)

        if answer('Is it OK? [y/n] '):
            logging.info('Selected %s option to work with.' % list[res_option])
            break

    while True:

        print('\nCurrent value of %s is %s.\n' % (list[res_option], Config.get(section, list[res_option])))
        logging.info('Current value of %s is %s.' % (list[res_option], Config.get(section, list[res_option])))
        new_value = raw_input('Please, set new value for %s: ' % list[res_option])
        print('Your new value for %s is %s.' % (list[res_option], new_value))
        logging.info('Entered new value %s.' % new_value)

        if answer('Is it OK? [y/n] '):
            Config.set(section, list[res_option], new_value)
            with open(file, 'r+') as c_file:
                Config.write(c_file)
                print('New value assigned successfully.')
                logging.info('New value assigned successfully. Finished at %s' % (time.ctime()))
            break

if __name__ == "__main__":
    try:
        edit_option()
    except KeyboardInterrupt:
        logging.info('Exited by KeyboardInterrupt at %s.' % (time.ctime()))
        print('\n')
        sys.exit(0)

And its execution:

$ ./setting.py

Working on DEV and using configuration directory /home/user/secure.

...
4 test.property
...

Please, select file to edit: 4
You selected: test.property
Is it OK? [y/n] y

...
1 Connection settings
...

Please, select section to edit: 1
You selected: Connection settings
Is it OK? [y/n] y

0 jdbc.main.url
...

Please, select option to edit: 1
You selected: jdbc.main.password
Is it OK? [y/n] n
0 jdbc.main.url
1 jdbc.main.password
2 jdbc.main.username

Please, select option to edit: 0
You selected: jdbc.main.url
Is it OK? [y/n] y

Current value of jdbc.main.url is jdbc:oracle:thin:@host1.

Please, set new value for jdbc.main.url: host2
Your new value for jdbc.main.url is host2.
Is it OK? [y/n] y
New value assigned successfully.

This is my first time using ConfigParser, so can something in its usage here be done better?

Also - I don't very like repeat the same loops a few times (when selecting files and sections and options) - can it be done other way? Move to separate function? It's difference in some parts...

Any other remarks?

share|improve this question

1 Answer 1

up vote 5 down vote accepted

select_conf

In the while loop, you are going through the directory and printing out a list of all the file in the directory so that the user can select the configuration file that they would like to work with.

You are going to end up doing this every time you go through the loop. It would be a better idea to stick this right before the loop so you aren't re-scanning the directory for files each time.

What if the user makes a mistake in their typing and accidently enters invalid input? Instead of exiting the program, you should just run the loop and ask for input again.

print('%s %s' % ((list.index(file)), file))

I believe that the first %s should be a %d or a %i because you are formatting a number, not a string.

select_section

Same things as last section: don't just exit the program if the user enters invalid input; loop again and give them another chance. Also, put the scanning for sections outside the loop for the same reason as above.

edit_option

Could this fail?

Config.read(file)

You did some error catching around this call in select_section, but you didn't do it here.

Same things go for this while loop as for the above ones.

The repeat loops

You expressed a concern for these in your post.

Yes, you could move these to a function, but the function would require copious areguments.

Here is what I tried:

def get_answer_loop(to_iterate, list, prompt, err_code):
    while True:
        for element in to_iterate:
            list.append(element)
            print('%s %s' % ((list.index(element)), section))

        try:
            choice = int(raw_input('\nPlease, ' + prompt + ': '))
            print('You selected: %s' % list[choice])
        except ValueError as e:
            print('Please, enter correct value: %s\n' %e)
            sys.exit(err_code) # either exit, or loop again as I recommended

        if answer('Is it OK? [y/n] '):
            logging.info('Selected %s section to work with.' % list[choice])
            return(list[choice], element) #element if needed; return if needed

Please excuse the naming.

General

  • Near the top of your code or in documentation about the application, you should create a list of all the exit codes and what errors go along with them.

  • At the end of those while loops where the user selects something, you don't need to have a break after a return().

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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