From 8418fb60837463479bfbbf65f6626a98accbf367 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Mon, 12 Jan 2015 21:47:44 +0100 Subject: [PATCH 1/5] Use a standard logger for the import log The import log now relies on a standard logger, named 'beets.importer' and configured upon initialization of the import session. --- beets/importer.py | 27 +++++++++++++++++---------- beets/ui/commands.py | 23 +++++++---------------- test/_common.py | 4 ++-- test/helper.py | 2 +- test/test_importer.py | 9 ++++++--- test/test_ui_importer.py | 2 +- 6 files changed, 34 insertions(+), 33 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 2ee143e4c..0800f7b96 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -26,6 +26,7 @@ from tempfile import mkdtemp from bisect import insort, bisect_left from contextlib import contextmanager import shutil +import time from beets import logging from beets import autotag @@ -174,14 +175,13 @@ class ImportSession(object): """Controls an import action. Subclasses should implement methods to communicate with the user or otherwise make decisions. """ - def __init__(self, lib, logfile, paths, query): - """Create a session. `lib` is a Library object. `logfile` is a - file-like object open for writing or None if no logging is to be - performed. Either `paths` or `query` is non-null and indicates + def __init__(self, lib, loghandler, paths, query): + """Create a session. `lib` is a Library object. `loghandler` is a + logging.Handler. Either `paths` or `query` is non-null and indicates the source of files to be imported. """ self.lib = lib - self.logfile = logfile + self.logger = self._setup_logging(loghandler) self.paths = paths self.query = query self.seen_idents = set() @@ -191,6 +191,15 @@ class ImportSession(object): if self.paths: self.paths = map(normpath, self.paths) + def _setup_logging(self, loghandler): + logger = logging.getLogger(__name__) + logger.propagate = False + if not loghandler: + log.info(u"Importer progress won't be logged") + loghandler = logging.NullHandler() + logger.handlers = [loghandler] + return logger + def set_config(self, config): """Set `config` property from global import config and make implied changes. @@ -225,13 +234,10 @@ class ImportSession(object): self.want_resume = config['resume'].as_choice([True, False, 'ask']) def tag_log(self, status, paths): - """Log a message about a given album to logfile. The status should + """Log a message about a given album to the log file. The status should reflect the reason the album couldn't be tagged. """ - if self.logfile: - print(u'{0} {1}'.format(status, displayable_path(paths)), - file=self.logfile) - self.logfile.flush() + self.logger.info(u'{0} {1}', status, displayable_path(paths)) def log_choice(self, task, duplicate=False): """Logs the task's current choice if it should be logged. If @@ -269,6 +275,7 @@ class ImportSession(object): def run(self): """Run the import task. """ + self.logger.info(u'import started {0}', time.asctime()) self.set_config(config['import']) # Set up the pipeline. diff --git a/beets/ui/commands.py b/beets/ui/commands.py index da96b1898..2c0863b60 100644 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -18,8 +18,6 @@ interface. from __future__ import print_function import os -import time -import codecs import platform import re import shlex @@ -825,29 +823,22 @@ def import_files(lib, paths, query): # Open the log. if config['import']['log'].get() is not None: - logpath = config['import']['log'].as_filename() + logpath = syspath(config['import']['log'].as_filename()) try: - logfile = codecs.open(syspath(logpath), 'a', 'utf8') + loghandler = logging.FileHandler(logpath) except IOError: - raise ui.UserError(u"could not open log file for writing: %s" % - displayable_path(logpath)) - print(u'import started', time.asctime(), file=logfile) + raise ui.UserError(u"could not open log file for writing: " + u"{0}".format(displayable_path(loghandler))) else: - logfile = None + loghandler = None # Never ask for input in quiet mode. if config['import']['resume'].get() == 'ask' and \ config['import']['quiet']: config['import']['resume'] = False - session = TerminalImportSession(lib, logfile, paths, query) - try: - session.run() - finally: - # If we were logging, close the file. - if logfile: - print(u'', file=logfile) - logfile.close() + session = TerminalImportSession(lib, loghandler, paths, query) + session.run() # Emit event. plugins.send('import', lib=lib, paths=paths) diff --git a/test/_common.py b/test/_common.py index 3852ba2f0..6107601c8 100644 --- a/test/_common.py +++ b/test/_common.py @@ -115,9 +115,9 @@ def album(lib=None): # Dummy import session. -def import_session(lib=None, logfile=None, paths=[], query=[], cli=False): +def import_session(lib=None, loghandler=None, paths=[], query=[], cli=False): cls = commands.TerminalImportSession if cli else importer.ImportSession - return cls(lib, logfile, paths, query) + return cls(lib, loghandler, paths, query) # A test harness for all beets tests. diff --git a/test/helper.py b/test/helper.py index afa5d29d1..af63c18a4 100644 --- a/test/helper.py +++ b/test/helper.py @@ -255,7 +255,7 @@ class TestHelper(object): config['import']['autotag'] = False config['import']['resume'] = False - return TestImportSession(self.lib, logfile=None, query=None, + return TestImportSession(self.lib, loghandler=None, query=None, paths=[import_dir]) # Library fixtures methods diff --git a/test/test_importer.py b/test/test_importer.py index 9a555c665..03b99a560 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -33,6 +33,7 @@ from beets.mediafile import MediaFile from beets import autotag from beets.autotag import AlbumInfo, TrackInfo, AlbumMatch from beets import config +from beets import logging class AutotagStub(object): @@ -209,7 +210,7 @@ class ImportHelper(TestHelper): config['import']['link'] = link self.importer = TestImportSession( - self.lib, logfile=None, query=None, + self.lib, loghandler=None, query=None, paths=[import_dir or self.import_dir] ) @@ -1219,13 +1220,15 @@ class ImportDuplicateSingletonTest(unittest.TestCase, TestHelper): class TagLogTest(_common.TestCase): def test_tag_log_line(self): sio = StringIO.StringIO() - session = _common.import_session(logfile=sio) + handler = logging.StreamHandler(sio) + session = _common.import_session(loghandler=handler) session.tag_log('status', 'path') assert 'status path' in sio.getvalue() def test_tag_log_unicode(self): sio = StringIO.StringIO() - session = _common.import_session(logfile=sio) + handler = logging.StreamHandler(sio) + session = _common.import_session(loghandler=handler) session.tag_log('status', 'caf\xc3\xa9') assert 'status caf' in sio.getvalue() diff --git a/test/test_ui_importer.py b/test/test_ui_importer.py index 8006e4215..0e3599301 100644 --- a/test/test_ui_importer.py +++ b/test/test_ui_importer.py @@ -91,7 +91,7 @@ class TerminalImportSessionSetup(object): self.io = DummyIO() self.io.install() self.importer = TestTerminalImportSession( - self.lib, logfile=None, query=None, io=self.io, + self.lib, loghandler=None, query=None, io=self.io, paths=[import_dir or self.import_dir], ) From 621ea60af4022f180e2ee478d36ad3166dfc9690 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Mon, 12 Jan 2015 22:08:11 +0100 Subject: [PATCH 2/5] Improve importer log unicode-handling test Send unicode instead of utf8-encoded string and check that the non-ASCII char is correctly handled. Bonus: use unittest.TestCase.assertIn(A, B) instead of "assert A in B". --- test/test_importer.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/test_importer.py b/test/test_importer.py index 03b99a560..7eab84e3e 100644 --- a/test/test_importer.py +++ b/test/test_importer.py @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- # This file is part of beets. # Copyright 2015, Adrian Sampson. # @@ -1223,14 +1224,14 @@ class TagLogTest(_common.TestCase): handler = logging.StreamHandler(sio) session = _common.import_session(loghandler=handler) session.tag_log('status', 'path') - assert 'status path' in sio.getvalue() + self.assertIn('status path', sio.getvalue()) def test_tag_log_unicode(self): sio = StringIO.StringIO() handler = logging.StreamHandler(sio) session = _common.import_session(loghandler=handler) - session.tag_log('status', 'caf\xc3\xa9') - assert 'status caf' in sio.getvalue() + session.tag_log('status', u'café') # send unicode + self.assertIn(u'status café', sio.getvalue()) class ResumeImportTest(unittest.TestCase, TestHelper): From cce0a5d81fc15641968323f6cc1fbfc804495103 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Tue, 13 Jan 2015 09:12:50 +0100 Subject: [PATCH 3/5] ImporterSession.tag_log(): improve docstring --- beets/importer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/beets/importer.py b/beets/importer.py index 0800f7b96..e0957e364 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -234,8 +234,8 @@ class ImportSession(object): self.want_resume = config['resume'].as_choice([True, False, 'ask']) def tag_log(self, status, paths): - """Log a message about a given album to the log file. The status should - reflect the reason the album couldn't be tagged. + """Log a message about a given album to the importer log. The status + should reflect the reason the album couldn't be tagged. """ self.logger.info(u'{0} {1}', status, displayable_path(paths)) From 81754e576062fffeb4a4ecb899fa5b5a53313a72 Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Tue, 13 Jan 2015 09:20:29 +0100 Subject: [PATCH 4/5] beets.logging exports NullHandler on python 2.6 NullHandler is not available in python 2.6. We backport it so the importer log can use it for it is more convenient than guarding calls to self.logger (see beets/importer.py) --- beets/logging.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/beets/logging.py b/beets/logging.py index dd1a28cf2..933fdb167 100644 --- a/beets/logging.py +++ b/beets/logging.py @@ -92,3 +92,16 @@ if PY26: return logger my_manager.getLogger = new_getLogger + + +# Offer NullHandler in Python 2.6 to reduce the difference with never versions +if PY26: + class NullHandler(Handler): + def handle(self, record): + pass + + def emit(self, record): + pass + + def createLock(self): + self.lock = None From 2cf327e0fd000d9b3426b9ddc66e35ce3237f6da Mon Sep 17 00:00:00 2001 From: Bruno Cauet Date: Tue, 13 Jan 2015 09:23:55 +0100 Subject: [PATCH 5/5] beet.importer: remove unnecessary log.info() call --- beets/importer.py | 1 - 1 file changed, 1 deletion(-) diff --git a/beets/importer.py b/beets/importer.py index e0957e364..deb13fd24 100644 --- a/beets/importer.py +++ b/beets/importer.py @@ -195,7 +195,6 @@ class ImportSession(object): logger = logging.getLogger(__name__) logger.propagate = False if not loghandler: - log.info(u"Importer progress won't be logged") loghandler = logging.NullHandler() logger.handlers = [loghandler] return logger