Python script to manage elasticsearch backups

The following is a Python 3 script I wrote to take and delete elasticsearch snapshots. Could anyone please point out any issues? I am concerned that I have structured the code poorly (particularly the order of the functions), making it hard to read, but I am unsure how to fix that.

I also made no real attempt to write pythonic code. Any ways I could improve the code to be more pythonic would also be appreciated.

GitHub

```
#!/usr/bin/env python3

def main():
import argparse, logging

# Parse arguments
parser = argparse.ArgumentParser(description='Take subcommands and \
parameters for elasticsearch backup script.')
parser.add_argument('function', type=str, choices=['backup','delete'],
help='Triggers a new elasticsearch action.')
parser.add_argument('--config', type=str,
help='Specify path to a config file.')
parser.add_argument('--name', type=str, help='Specify snapshot name')
parser.add_argument('--age', type=int,
help='Specify age to delete backups after')
parser.add_argument('--logfile', type=str,
help='Specify where to put a logfile')
parser.add_argument('--loglevel', type=str, help='Specify log level',
choices=['INFO', 'DEBUG', 'WARNING', 'ERROR', 'CRITICAL'])

args = parser.parse_args()

# Find config file, and read in config
config = find_config(args.config)

# Check if logfile was specified
if args.logfile == None:
logfile = '/var/log/elasticsearch/snapshot_backup.log'
else:
logfile = args.logfile

# Check if logging level was specified
try:
if args.loglevel != None:
logging_level = logging.getLevelName(args.loglevel)
print('I am args: {0}'.format(logging_level))
elif config['logging_level'] != None:
logging_level = logging.getLevelName(config['logging_level'])
print('I am config: {0}'.format(logging_level))
except KeyError as e:
logging_level = logging.getLevelName('ERROR')

You have at-least the following problems:

• You should put all your imports at the top of your file, just after your shebang. What you're doing makes it hard to know what you've actually imported and causes imports to be more tedious that they're meant to be.

• Argparse allows you to set defaults, and so you don't need to implement this yourself.

• You should use is when comparing to singletons. And so you should use if cond is None: rather than if cond == None:. You've done this 7 times.

• You don't use some of your imports. datetime and re on line 215 and 149 respectively.

• It's advised against in PEP8 to import multiple modules in one import.

• json is never defined in bulk_delete, or the global scope. And so shouldn't work in this function.

• It's hard to read your add_argument calls, as you put help on the following line without extra indentation, and so looks, at a glance, that you're constantly re-defining a help variable.

• PEP8 advices that module level functions and classes should have two empty lines between them. This IMO makes the code cleaner and easier to read at a glance.

• You should move your large try-except blocks to be in a single request function. This is as they take up a large portion of your program. And make your code WET, which is not good. As if you ever need to change how the excepts work, you need to change all the occurrences.

-
Setting defaults are easier if you overwrite a variable if it's bad with the default. Take configfile in find_config, you could instead do:

def find_config(config_file):
    if not config_file:
        config_file = '/etc/elasticsearch/backup.yaml'

    try:

You should look into reading PEP8, and use a linter to follow the parts that you agree with.

And I think the way you are logging is promoting you to write bad code. And so I'd advise you write your code without logging, and then add it in afterwords.

StackExchange Code Review Q#155972, answer score: 6