patternpythonMinor
OVH email manager script using docopt
Viewed 0 times
scriptdocoptemailusingmanagerovh
Problem
This is what I coded to realize a command line program to make it easier to manage emails on ovh shared hostings. It's working ok but it seems ugly.
I'm a beginner with Python and
```
#!/usr/bin/env python
# -- coding: utf-8 --
''' OVH Email Manager (ovhEmailMan)
A small script that helps to add and remove one or more email addresses on the OVH shared domains
Usage:
ovh_mails.py list [--ugly]
ovh_mails.py add ( [--pswd=][--description=] | --file [--notify])
ovh_mails.py remove ( | --file )
ovh_mails.py (-h | --help)
Arguments:
Password to access the mailbox (if not provided it's random generated)
Name of the files to process (csv). Check README to see how to format it
Options:
-h, --help Show this help message
-u, --ugly Print without nice tables
-p, --pswd= Set the password to the one provided
-n, --notify If set, notification mail is sent using smtp credentials in ovh.conf
Commands:
list list all the email addresses currently configured
add add one or more (configured in ) email addresses
remove remove one ore more (configured in ) email addresses
'''
import ovh
from docopt import docopt
from ovhem import EmailManager
from ovhem import fileprocesser as fp
if __name__ == '__main__':
args = docopt(__doc__)
#Validate args ---- TODO
eman = EmailManager()
# 'List' command parsing
if args['list']:
if args['--ugly']:
eman.niceoutput = False
eman.list_emails()
# 'Add' command parsing
elif args['add']:
if args['']:
emails = (
{
'address': args[''],
'pa
I'm a beginner with Python and
docopt. What would you suggest?```
#!/usr/bin/env python
# -- coding: utf-8 --
''' OVH Email Manager (ovhEmailMan)
A small script that helps to add and remove one or more email addresses on the OVH shared domains
Usage:
ovh_mails.py list [--ugly]
ovh_mails.py add ( [--pswd=][--description=] | --file [--notify])
ovh_mails.py remove ( | --file )
ovh_mails.py (-h | --help)
Arguments:
Password to access the mailbox (if not provided it's random generated)
Name of the files to process (csv). Check README to see how to format it
Options:
-h, --help Show this help message
-u, --ugly Print without nice tables
-p, --pswd= Set the password to the one provided
-n, --notify If set, notification mail is sent using smtp credentials in ovh.conf
Commands:
list list all the email addresses currently configured
add add one or more (configured in ) email addresses
remove remove one ore more (configured in ) email addresses
'''
import ovh
from docopt import docopt
from ovhem import EmailManager
from ovhem import fileprocesser as fp
if __name__ == '__main__':
args = docopt(__doc__)
#Validate args ---- TODO
eman = EmailManager()
# 'List' command parsing
if args['list']:
if args['--ugly']:
eman.niceoutput = False
eman.list_emails()
# 'Add' command parsing
elif args['add']:
if args['']:
emails = (
{
'address': args[''],
'pa
Solution
Overall, it looks quite fine, with some smaller and bigger issues.
Parsing the
It's not so great to use the
You might mistype one of them. Or you might change something later and forget to update it everywhere. It would be better to put the value of
This part is also awkward:
It would be much easier to use
Like this:
Parsing the
Here, if the second
it will overwrite the effect of the first:
That suggest that you should use an
Both the parsing of
As their last step they do something with
for example
but at that point the
This can be fixed by adding validation for these commands.
It looks that either of these must be true, checked in this order:
For example:
Parsing the
add commandIt's not so great to use the
'' string twice here:if args['']:
emails = (
{
'address': args[''],
# ...You might mistype one of them. Or you might change something later and forget to update it everywhere. It would be better to put the value of
args[''] into a variable, and use that variable in evaluations and assignments.This part is also awkward:
emails = ( ... )
if args['--description']:
emails[0]['description'] = args['']
if args['--pswd']:
emails[0]['password'] = args['']It would be much easier to use
description and password variables to store defaults, then update these variables if --description or --pswd were given, and finally create the emails tuple.Like this:
address = args['']
if address:
password = None
description = None
if args['--description']:
description = args['']
if args['--pswd']:
password = args['']
emails = (
{
'address': address,
'password': password,
'description': description,
},
)Parsing the
remove commandHere, if the second
if statement is true,it will overwrite the effect of the first:
if args['']:
emails = (
{
'address': args[''],
},
)
if args['--file']:
emails = fp.process_file(args[''])That suggest that you should use an
elif, and switch the order of the statements:if args['--file']:
emails = fp.process_file(args[''])
elif args['']:
emails = (
{
'address': args[''],
},
)emails might be undefinedBoth the parsing of
add and remove have a problem:As their last step they do something with
emails,for example
eman.add_emails(emails) and eman.remove_emails(emails),but at that point the
emails variable might be undefined.This can be fixed by adding validation for these commands.
It looks that either of these must be true, checked in this order:
--filewas specified and `parameter was given
- ` parameter was given
For example:
if args['--file']:
filename = args['']
if filename:
emails = fp.process_file(filename)
else:
raise Exception("Filename parameter missing")
else:
address = args['']
if address:
emails = ( ... )
else:
raise Exception("Address parameter missing")
# main actionCode Snippets
if args['<address>']:
emails = (
{
'address': args['<address>'],
# ...emails = ( ... )
if args['--description']:
emails[0]['description'] = args['<description>']
if args['--pswd']:
emails[0]['password'] = args['<password>']address = args['<address>']
if address:
password = None
description = None
if args['--description']:
description = args['<description>']
if args['--pswd']:
password = args['<password>']
emails = (
{
'address': address,
'password': password,
'description': description,
},
)if args['<address>']:
emails = (
{
'address': args['<address>'],
},
)
if args['--file']:
emails = fp.process_file(args['<filename>'])if args['--file']:
emails = fp.process_file(args['<filename>'])
elif args['<address>']:
emails = (
{
'address': args['<address>'],
},
)Context
StackExchange Code Review Q#82784, answer score: 2
Revisions (0)
No revisions yet.