[PATCH v4 1/2] parsemail: Convert to a management command

Andrew Donnellan andrew.donnellan at au1.ibm.com
Mon Sep 5 14:22:06 AEST 2016


On 05/09/16 13:58, Andrew Donnellan wrote:
>> +    def handle(self, *args, **options):
>> +        path = (args[0] if args else
>> +                options['infile'] if 'infile' in options else None)
>
> argparse is giving you a file, not a string. As you've given a default
> in the argument, this should always return an open file, I'm pretty sure.
>
>> +        stdin = options.get('stdin', sys.stdin)
>
> What's this for?
>
>> +
>> +        # Attempt to parse the path if provided, and fallback to
>> stdin if not
>> +        if path and not isinstance(path, file):
>> +            logger.info('Parsing mail loaded by filename')
>> +            with open(path, 'r+') as file_:
>> +                mail = message_from_file(file_)
>> +        else:
>> +            logger.info('Parsing mail loaded from stdin')
>> +            mail = message_from_file(stdin)
>> +
>
> As noted in my other email this is all broken.
>
> I'd rewrite all this as follows (lightly tested on Py2 and 3):
>
>     def handle(self, *args, **options):
>        infile = options['infile']
>        if infile == sys.stdin:
>            logger.info('Parsing mail loaded from stdin')
>        else:
>            logger.info('Parsing mail loaded by filename')
>        mail = message_from_file(infile)

Ahh, I forgot about the optparse case here. It'll be wonderful once we 
can finally dump Django 1.6...

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan at au1.ibm.com  IBM Australia Limited



More information about the Patchwork mailing list