Skip to content

Commit

Permalink
argparse & logging (#92)
Browse files Browse the repository at this point in the history
* introduce logcommand.Lager, Whipper(); use argparse for whipper image commands, stub logging

* update Lager docstring to mention config.Config()

* make incorrect subcommand and --version work on toplevel command

* migrate accurip show, expand Lager, do not attempt to return from Lager.__init__.

* migrate offset find, add Lager.error

* correct offset find drive symlink handling

* migrate drive

* change Lager.__init__(prog) to arg from kwarg

* but actually

* remove Whipper.usage

* add and use Lager.device_option() context manager

* help I married an axe murderer

* use unified options namespace for entire command tree

* migrate whipper cd without comprehensive config loading

* switch to logging module

- use logging instead of flog for non-extern modules
- use WHIPPER_DEBUG and WHIPPER_LOGFILE env variables

* convert self.log calls to logger.debug

* convert self.error calls to logger.error

* remove log.Loggable, use logger not logging

* Logging conversion continues

- Convert log.* calls to logger.*
- Remove morituri.common.log imports

* remove morituri.common.log from tests

* remove extern/flog, bare minimum Debug conversion

* update README for logging changes

* update soxi to use logging

* refactor Lager for more declarative subcommands

* Refactor Lager.device_option:

- inline into __init__
- throw IOError instead of Exception for missing drives
- remove CommandError checking in rip/main

* rename rip to whipper in rip.main

* convert rip.debug commands

* Rename logcommand.Lager to command.BaseCommand

- remove command.CommandError occurrences
- remove python-command external module

* remove submodules from README, update rclog formatter

* update minor ambiguity in readme for command invocation

* update version number to match setup.py

* remove gitmodules

* update version number in tests as well (boo)

* convert logger.error to logger.critical

* Change morituri.rip to morituri.command

- mv common.command to command.basecommand
- move TEMPLATES used only by rip.cd out of rip.common
- update entry point for command to command.main

* update basecommand documentation

* go pyflaking: import fixing

* replace self.stdout with sys.stdout

* remove BaseCommand.config, alphabetise imports

* convert self.stdXXX leftovers

* convert last getRootCommand to config.Config

* convert last getExceptionMessage's to str

* change musicbrainz useragent to whipper
  • Loading branch information
RecursiveForest authored and JoeLametta committed Dec 20, 2016
1 parent 8f4607d commit d1ed80d
Show file tree
Hide file tree
Showing 48 changed files with 1,049 additions and 1,072 deletions.
6 changes: 0 additions & 6 deletions .gitmodules

This file was deleted.

11 changes: 5 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,6 @@ Change to a directory where you want to put whipper source code (for example, `$
```bash
git clone -b master --single-branch https://github.com/JoeLametta/whipper.git
cd whipper
# fetch bundled python dependencies
git submodule init
git submodule update
```

### Building the bundled dependencies
Expand Down Expand Up @@ -114,7 +111,7 @@ is correct, while

`whipper cd rip -d (device)`

is not, because the `-d` argument applies to the whipper command.
is not, because the `-d` argument applies to the `cd` command.

~~Check the man page (`whipper(1)`) for more information.~~ (currently not available as whipper's documentation is planned to be reworked ([Issue #73](https://github.com/JoeLametta/whipper/issues/73)).

Expand Down Expand Up @@ -261,15 +258,17 @@ Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA

Please use the [issue tracker](https://github.com/JoeLametta/whipper/issues) to report any bugs or to file feature requests.

When filing bug reports, please run the failing command with the environment variable `RIP_DEBUG` set. For example:
When filing bug reports, please run the failing command with the environment variable `WHIPPER_DEBUG` set. For example:

```bash
RIP_DEBUG=5 whipper offset find > whipper.log 2>&1
WHIPPER_DEBUG=DEBUG WHIPPER_LOGFILE=whipper.log whipper offset find
gzip whipper.log
```

And attach the gzipped log file to your bug report.

Without `WHIPPER_LOGFILE` set, logging messages will go to stderr. `WHIPPER_DEBUG` accepts a string of the [default python logging levels](https://docs.python.org/2/library/logging.html#logging-levels).

### Developing

Pull requests are welcome.
Expand Down
12 changes: 12 additions & 0 deletions morituri/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import logging
import os
import sys

level = logging.WARNING
if 'WHIPPER_DEBUG' in os.environ:
level = os.environ['WHIPPER_DEBUG'].upper()
if 'WHIPPER_LOGFILE' in os.environ:
logging.basicConfig(filename=os.environ['WHIPPER_LOGFILE'],
filemode='w', level=level)
else:
logging.basicConfig(stream=sys.stderr, level=level)
File renamed without changes.
44 changes: 27 additions & 17 deletions morituri/rip/accurip.py → morituri/command/accurip.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,39 +20,44 @@
# You should have received a copy of the GNU General Public License
# along with morituri. If not, see <http://www.gnu.org/licenses/>.

from morituri.common import logcommand, accurip
import sys

from morituri.command.basecommand import BaseCommand
from morituri.common import accurip

class Show(logcommand.LogCommand):
import logging
logger = logging.getLogger(__name__)

class Show(BaseCommand):
summary = "show accuraterip data"
description = """
retrieves and display accuraterip data from the given URL
"""

def do(self, args):

try:
url = args[0]
except IndexError:
self.stdout.write('Please specify an accuraterip URL.\n')
return 3
def add_arguments(self):
self.parser.add_argument('url', action='store',
help="accuraterip URL to load data from")

def do(self):
url = self.options.url
cache = accurip.AccuCache()
responses = cache.retrieve(url)

count = responses[0].trackCount

self.stdout.write("Found %d responses for %d tracks\n\n" % (
sys.stdout.write("Found %d responses for %d tracks\n\n" % (
len(responses), count))

for (i, r) in enumerate(responses):
if r.trackCount != count:
self.stdout.write(
sys.stdout.write(
"Warning: response %d has %d tracks instead of %d\n" % (
i, r.trackCount, count))


# checksum and confidence by track
for track in range(count):
self.stdout.write("Track %d:\n" % (track + 1))
sys.stdout.write("Track %d:\n" % (track + 1))
checksums = {}

for (i, r) in enumerate(responses):
Expand Down Expand Up @@ -81,12 +86,17 @@ def do(self, args):
sortedChecksums.reverse()

for highest, checksum in sortedChecksums:
self.stdout.write(" %d result(s) for checksum %s: %s\n" % (
sys.stdout.write(" %d result(s) for checksum %s: %s\n" % (
len(checksums[checksum]), checksum,
str(checksums[checksum])))


class AccuRip(logcommand.LogCommand):
description = "Handle AccurateRip information."

subCommandClasses = [Show, ]
class AccuRip(BaseCommand):
summary = "handle AccurateRip information"
description = """
Handle AccurateRip information. Retrieves AccurateRip disc entries and
displays diagnostic information.
"""
subcommands = {
'show': Show
}
129 changes: 129 additions & 0 deletions morituri/command/basecommand.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
# -*- Mode: Python -*-
# vi:si:et:sw=4:sts=4:ts=4

import argparse
import os
import sys

from morituri.common import drive

import logging
logger = logging.getLogger(__name__)

# Q: What about argparse.add_subparsers(), you ask?
# A: Unfortunately add_subparsers() does not support specifying the
# formatter_class of subparsers, nor does it support epilogs, so
# it does not quite fit our use case.

# Q: Why not subclass ArgumentParser and extend/replace the relevant
# methods?
# A: If this can be done in a simpler fashion than this current
# implementation, by all means submit a patch.

# Q: Why not argparse.parse_known_args()?
# A: The prefix matching prevents passing '-h' (and possibly other
# options) to the child command.

class BaseCommand():
"""
A base command class for whipper commands.
Creates an argparse.ArgumentParser.
Override add_arguments() and handle_arguments() to register
and process arguments before & after argparse.parse_args().
Provides self.epilog() formatting command for argparse.
device_option = True adds -d / --device option to current command
no_add_help = True removes -h / --help option from current command
Overriding formatter_class sets the argparse formatter class.
If the 'subcommands' dictionary is set, __init__ searches the
arguments for subcommands.keys() and instantiates the class
implementing the subcommand as self.cmd, passing all non-understood
arguments, the current options namespace, and the full command path
name.
"""
device_option = False
no_add_help = False # for rip.main.Whipper
formatter_class = argparse.RawDescriptionHelpFormatter

def __init__(self, argv, prog_name, opts):
self.opts = opts # for Rip.add_arguments()
self.prog_name = prog_name

self.init_parser()
self.add_arguments()

if hasattr(self, 'subcommands'):
self.parser.add_argument('remainder',
nargs=argparse.REMAINDER,
help=argparse.SUPPRESS)

if self.device_option:
# pick the first drive as default
drives = drive.getAllDevicePaths()
if not drives:
msg = 'No CD-DA drives found!'
logger.critical(msg)
# morituri exited with return code 3 here
raise IOError(msg)
self.parser.add_argument('-d', '--device',
action="store",
dest="device",
default=drives[0],
help="CD-DA device")

self.options = self.parser.parse_args(argv, namespace=opts)

if self.device_option:
# this can be a symlink to another device
self.options.device = os.path.realpath(self.options.device)
if not os.path.exists(self.options.device):
msg = 'CD-DA device %s not found!' % self.options.device
logger.critical(msg)
raise IOError(msg)

self.handle_arguments()

if hasattr(self, 'subcommands'):
if not self.options.remainder:
self.parser.print_help()
sys.exit(0)
if not self.options.remainder[0] in self.subcommands:
sys.stderr.write("incorrect subcommand: %s" %
self.options.remainder[0])
sys.exit(1)
self.cmd = self.subcommands[self.options.remainder[0]](
self.options.remainder[1:],
prog_name + " " + self.options.remainder[0],
self.options
)

def init_parser(self):
kw = {
'prog': self.prog_name,
'description': self.description,
'formatter_class': self.formatter_class,
}
if hasattr(self, 'subcommands'):
kw['epilog'] = self.epilog()
if self.no_add_help:
kw['add_help'] = False
self.parser = argparse.ArgumentParser(**kw)

def add_arguments(self):
pass

def handle_arguments(self):
pass

def do(self):
return self.cmd.do()

def epilog(self):
s = "commands:\n"
for com in sorted(self.subcommands.keys()):
s += " %s %s\n" % (com.ljust(8), self.subcommands[com].summary)
return s
Loading

0 comments on commit d1ed80d

Please sign in to comment.