From 1481a05d7060c8cfd615850b3df9d66a52fb0ff4 Mon Sep 17 00:00:00 2001 From: kaliko Date: Wed, 28 Jan 2015 20:41:49 +0100 Subject: [PATCH] Improved CLI error handling --- sima/utils/config.py | 12 ++++++++++-- sima/utils/startopt.py | 18 +++++++----------- sima/utils/utils.py | 6 ++++-- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/sima/utils/config.py b/sima/utils/config.py index bf370f1..0e775ec 100644 --- a/sima/utils/config.py +++ b/sima/utils/config.py @@ -127,13 +127,19 @@ class ConfMan(object): # CONFIG MANAGER CLASS self.config['sima']['db_file'] = join(self.config['sima']['var_dir'], 'sima.db') def control_facc(self): - """TODO: redundant with startopt cli args controls + """Controls file access. + This is relevant only for file provided through the configuration file + since files provided on the command line are already checked with + argparse. """ ok = True for op, ftochk in [('log', self.config['log']['logfile']), ('pidfile', self.config['daemon']['pidfile']),]: if not ftochk: continue + if isdir(ftochk): + self.log.critical('Need a file not a directory: "{}"'.format(ftochk)) + ok = False if not exists(ftochk): # Is parent directory writable then filedir = dirname(ftochk) @@ -142,9 +148,11 @@ class ConfMan(object): # CONFIG MANAGER CLASS ok = False else: if not access(ftochk, W_OK): - self.log.critical('no write access to "{0}" ({1}))'.format(ftochk, op)) + self.log.critical('no write access to "{0}" ({1})'.format(ftochk, op)) ok = False if not ok: + if exists(self.conf_file): + self.log.warning('Try to check the configuration file: {}'.format(self.conf_file)) sys.exit(2) def control_mod(self): diff --git a/sima/utils/startopt.py b/sima/utils/startopt.py index ef2032c..51319f6 100644 --- a/sima/utils/startopt.py +++ b/sima/utils/startopt.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- -# Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Jack Kaliko +# Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Jack Kaliko # # This file is part of sima # @@ -24,12 +24,9 @@ from argparse import (ArgumentParser, SUPPRESS) from .utils import Wfile, Rfile, Wdir -USAGE = """USAGE: %prog [--help] [options]""" DESCRIPTION = """ -sima automagicaly queue new tracks in MPD playlist. -All command line options will override their equivalent in configuration -file. -""" +MPD_sima automagicaly queue new tracks in MPD playlist. +Command line options override their equivalent in configuration file.""" def clean_dict(to_clean): @@ -124,12 +121,11 @@ class StartOpt(object): Declare options in ArgumentParser object. """ self.parser = ArgumentParser(description=DESCRIPTION, - usage='%(prog)s [options]', - prog=self.info.get('prog'), - epilog='Happy Listening', - ) + prog=self.info.get('prog'), + epilog='Happy Listening', + ) self.parser.add_argument('--version', action='version', - version='%(prog)s {version}'.format(**self.info)) + version='%(prog)s {version}'.format(**self.info)) # Add all options declare in OPTS for opt in OPTS: opt_names = opt.pop('sw') diff --git a/sima/utils/utils.py b/sima/utils/utils.py index ff06410..2cb7c35 100644 --- a/sima/utils/utils.py +++ b/sima/utils/utils.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- # -# Copyright (c) 2010, 2011, 2013, 2014 Jack Kaliko +# Copyright (c) 2010, 2011, 2013, 2014, 2015 Jack Kaliko # # This file is part of sima # @@ -113,9 +113,11 @@ class Wfile(FileAction): """Is file writable """ def checks(self): + if isdir(self._file): + self.parser.error('need a file not a directory: {}'.format(self._file)) if not exists(self._dir): #raise ArgumentError(self, '"{0}" does not exist'.format(self._dir)) - self.parser.error('file does not exist: {0}'.format(self._dir)) + self.parser.error('directory does not exist: {0}'.format(self._dir)) if not exists(self._file): # Is parent directory writable then if not access(self._dir, W_OK): -- 2.39.5