From 1f422a9609ed80afff4914307dd058e9b52c24ea Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Wed, 15 Aug 2012 11:13:36 -0400 Subject: [PATCH 1/3] cc-4105: removed obsolete comments and duplicate code --- .../media/monitor/eventcontractor.py | 1 - .../media/monitor/watchersyncer.py | 21 +++++++------------ 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/python_apps/media-monitor2/media/monitor/eventcontractor.py b/python_apps/media-monitor2/media/monitor/eventcontractor.py index 0ac0e3278..cbc1f5c90 100644 --- a/python_apps/media-monitor2/media/monitor/eventcontractor.py +++ b/python_apps/media-monitor2/media/monitor/eventcontractor.py @@ -46,7 +46,6 @@ class EventContractor(Loggable): self.store[ evt.path ] = evt return True # We actually added something, hence we return true. - # events are unregistered automatically no need to screw around with them def __unregister(self, evt): try: del self.store[evt.path] except Exception as e: self.unexpected_exception(e) diff --git a/python_apps/media-monitor2/media/monitor/watchersyncer.py b/python_apps/media-monitor2/media/monitor/watchersyncer.py index faea2600d..c400ad6d5 100644 --- a/python_apps/media-monitor2/media/monitor/watchersyncer.py +++ b/python_apps/media-monitor2/media/monitor/watchersyncer.py @@ -2,7 +2,6 @@ import threading import time import copy -import traceback from media.monitor.handler import ReportHandler from media.monitor.log import Loggable @@ -47,9 +46,7 @@ class RequestSync(threading.Thread,Loggable): except BadSongFile as e: self.logger.info("This should never occur anymore!!!") self.logger.info("Bad song file: '%s'" % e.path) - except Exception as e: - self.logger.info("An evil exception occured") - self.logger.error( traceback.format_exc() ) + except Exception as e: self.unexpected_exception( e ) def make_req(): self.apiclient.send_media_monitor_requests( packed_requests ) for try_index in range(0,self.retries): @@ -102,16 +99,14 @@ class TimeoutWatcher(threading.Thread,Loggable): class WatchSyncer(ReportHandler,Loggable): def __init__(self, signal, chunking_number = 100, timeout=15): - self.timeout = float(timeout) - self.chunking_number = int(chunking_number) - self.__reset_queue() - # Even though we are not blocking on the http requests, we are still - # trying to send the http requests in order - self.__requests = [] - self.request_running = False - # we don't actually use this "private" instance variable anywhere + self.timeout = float(timeout) + self.chunking_number = int(chunking_number) + self.request_running = False self.__current_thread = None - self.contractor = EventContractor() + self.__requests = [] + self.contractor = EventContractor() + self.__reset_queue() + tc = TimeoutWatcher(self, self.timeout) tc.daemon = True tc.start() From e9114b3468abe5261c401dfa405fd7cd3e0adcc6 Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Wed, 15 Aug 2012 11:22:44 -0400 Subject: [PATCH 2/3] cc-4105: added docstrings, formatted code --- python_apps/media-monitor2/media/monitor/config.py | 7 +++++-- python_apps/media-monitor2/media/monitor/metadata.py | 4 +--- python_apps/media-monitor2/media/monitor/pure.py | 5 +++-- python_apps/media-monitor2/mm2.py | 2 ++ 4 files changed, 11 insertions(+), 7 deletions(-) diff --git a/python_apps/media-monitor2/media/monitor/config.py b/python_apps/media-monitor2/media/monitor/config.py index 0669800b3..c11158574 100644 --- a/python_apps/media-monitor2/media/monitor/config.py +++ b/python_apps/media-monitor2/media/monitor/config.py @@ -8,8 +8,7 @@ import media.monitor.pure as mmp class MMConfig(object): def __init__(self, path): - if not os.path.exists(path): - raise NoConfigFile(path) + if not os.path.exists(path): raise NoConfigFile(path) self.cfg = ConfigObj(path) def __getitem__(self, key): @@ -29,6 +28,10 @@ class MMConfig(object): def save(self): self.cfg.write() def last_ran(self): + """ + Returns the last time media monitor was ran by looking at the time when + the file at 'index_path' was modified + """ return mmp.last_modified(self.cfg['index_path']) # Remove this after debugging... diff --git a/python_apps/media-monitor2/media/monitor/metadata.py b/python_apps/media-monitor2/media/monitor/metadata.py index d1d35a99e..2a209d2bd 100644 --- a/python_apps/media-monitor2/media/monitor/metadata.py +++ b/python_apps/media-monitor2/media/monitor/metadata.py @@ -110,8 +110,7 @@ class Metadata(Loggable): """ Writes 'md' metadata into 'path' through mutagen """ - if not os.path.exists(path): - raise BadSongFile(path) + if not os.path.exists(path): raise BadSongFile(path) song_file = mutagen.File(path, easy=True) for airtime_k, airtime_v in md.iteritems(): if airtime_k in airtime2mutagen: @@ -120,7 +119,6 @@ class Metadata(Loggable): song_file[ airtime2mutagen[airtime_k] ] = unicode(airtime_v) song_file.save() - def __init__(self, fpath): # Forcing the unicode through try: fpath = fpath.decode("utf-8") diff --git a/python_apps/media-monitor2/media/monitor/pure.py b/python_apps/media-monitor2/media/monitor/pure.py index ceb37cd6c..77756ce87 100644 --- a/python_apps/media-monitor2/media/monitor/pure.py +++ b/python_apps/media-monitor2/media/monitor/pure.py @@ -17,6 +17,9 @@ supported_extensions = [u"mp3", u"ogg", u"oga"] #supported_extensions = [u"mp3", u"ogg", u"oga", u"flac", u"aac", u"bwf"] unicode_unknown = u'unknown' +path_md = ['MDATA_KEY_TITLE', 'MDATA_KEY_CREATOR', 'MDATA_KEY_SOURCE', + 'MDATA_KEY_TRACKNUMBER', 'MDATA_KEY_BITRATE'] + class LazyProperty(object): """ meant to be used for lazy evaluation of an object attribute. @@ -223,8 +226,6 @@ def normalized_metadata(md, original_path): 'MDATA_KEY_MIME' : lambda x: x.replace('-','/'), 'MDATA_KEY_BPM' : lambda x: x[0:8], } - path_md = ['MDATA_KEY_TITLE', 'MDATA_KEY_CREATOR', 'MDATA_KEY_SOURCE', - 'MDATA_KEY_TRACKNUMBER', 'MDATA_KEY_BITRATE'] # note that we could have saved a bit of code by rewriting new_md using # defaultdict(lambda x: "unknown"). But it seems to be too implicit and # could possibly lead to subtle bugs down the road. Plus the following diff --git a/python_apps/media-monitor2/mm2.py b/python_apps/media-monitor2/mm2.py index a8a3dfb26..849d7ae6b 100644 --- a/python_apps/media-monitor2/mm2.py +++ b/python_apps/media-monitor2/mm2.py @@ -122,6 +122,8 @@ def main(global_config, api_client_config, log_config, # Launch the toucher that updates the last time when the script was # ran every n seconds. + # TODO : verify that this does not interfere with bootstrapping because the + # toucher thread might update the last_ran variable too fast tt = ToucherThread(path=config['index_path'], interval=int(config['touch_interval'])) From 71b62608e5ffae54bc0fd66eee9db053aadd325c Mon Sep 17 00:00:00 2001 From: Rudi Grinberg Date: Wed, 15 Aug 2012 12:02:51 -0400 Subject: [PATCH 3/3] cc-4226: fixed empty fields to be shown as empty strings instead of placeholder unknown. updated tests --- .../media-monitor2/media/monitor/organizer.py | 8 ++--- .../media-monitor2/media/monitor/pure.py | 35 ++++++++++++------- python_apps/media-monitor2/tests/test_pure.py | 7 ++++ 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/python_apps/media-monitor2/media/monitor/organizer.py b/python_apps/media-monitor2/media/monitor/organizer.py index 011383c39..e5d0247de 100644 --- a/python_apps/media-monitor2/media/monitor/organizer.py +++ b/python_apps/media-monitor2/media/monitor/organizer.py @@ -18,8 +18,8 @@ class Organizer(ReportHandler,Loggable): _instance = None def __new__(cls, channel, target_path, recorded_path): if cls._instance: - cls._instance.channel = channel - cls._instance.target_path = target_path + cls._instance.channel = channel + cls._instance.target_path = target_path cls._instance.recorded_path = recorded_path else: cls._instance = super(Organizer, cls).__new__( cls, channel, @@ -27,8 +27,8 @@ class Organizer(ReportHandler,Loggable): return cls._instance def __init__(self, channel, target_path, recorded_path): - self.channel = channel - self.target_path = target_path + self.channel = channel + self.target_path = target_path self.recorded_path = recorded_path super(Organizer, self).__init__(signal=self.channel, weak=False) diff --git a/python_apps/media-monitor2/media/monitor/pure.py b/python_apps/media-monitor2/media/monitor/pure.py index 77756ce87..7160f18c0 100644 --- a/python_apps/media-monitor2/media/monitor/pure.py +++ b/python_apps/media-monitor2/media/monitor/pure.py @@ -166,15 +166,23 @@ def apply_rules_dict(d, rules): if k in d: new_d[k] = rule(d[k]) return new_d +def default_to_f(dictionary, keys, default, condition): + new_d = copy.deepcopy(dictionary) + for k in keys: + if condition(dictionary=new_d, key=k): new_d[k] = default + return new_d + def default_to(dictionary, keys, default): """ Checks if the list of keys 'keys' exists in 'dictionary'. If not then it returns a new dictionary with all those missing keys defaults to 'default' """ - new_d = copy.deepcopy(dictionary) - for k in keys: - if not (k in new_d): new_d[k] = default - return new_d + cnd = lambda dictionary, key: key not in dictionary + return default_to_f(dictionary, keys, default, cnd) + #new_d = copy.deepcopy(dictionary) + #for k in keys: + #if not (k in new_d): new_d[k] = default + #return new_d def remove_whitespace(dictionary): """ @@ -234,8 +242,9 @@ def normalized_metadata(md, original_path): new_md = apply_rules_dict(new_md, format_rules) new_md = default_to(dictionary=new_md, keys=['MDATA_KEY_TITLE'], default=no_extension_basename(original_path)) + new_md = remove_whitespace(new_md) new_md = default_to(dictionary=new_md, keys=path_md, - default=unicode_unknown) + default=u'') new_md = default_to(dictionary=new_md, keys=['MDATA_KEY_FTYPE'], default=u'audioclip') # In the case where the creator is 'Airtime Show Recorder' we would like to @@ -252,13 +261,13 @@ def normalized_metadata(md, original_path): # be set to the original path of the file for airtime recorded shows # (before it was "organized"). We will skip this procedure for now # because it's not clear why it was done - return remove_whitespace(new_md) + return new_md -def organized_path(old_path, root_path, normal_md): +def organized_path(old_path, root_path, orig_md): """ old_path - path where file is store at the moment <= maybe not necessary? root_path - the parent directory where all organized files go - normal_md - original meta data of the file as given by mutagen AFTER being + orig_md - original meta data of the file as given by mutagen AFTER being normalized return value: new file path """ @@ -266,6 +275,8 @@ def organized_path(old_path, root_path, normal_md): ext = extension(old_path) # The blocks for each if statement look awfully similar. Perhaps there is a # way to simplify this code + normal_md = default_to_f(orig_md, path_md, unicode_unknown, + lambda dictionary, key: len(dictionary[key]) == 0) if is_airtime_recorded(normal_md): fname = u'%s-%s-%s.%s' % ( normal_md['MDATA_KEY_YEAR'], normal_md['MDATA_KEY_TITLE'], @@ -322,8 +333,7 @@ def get_system_locale(locale_path='/etc/default/locale'): try: config = ConfigObj(locale_path) return config - except Exception as e: - raise FailedToSetLocale(locale_path,cause=e) + except Exception as e: raise FailedToSetLocale(locale_path,cause=e) else: raise ValueError("locale path '%s' does not exist. \ permissions issue?" % locale_path) @@ -337,8 +347,7 @@ def configure_locale(config): if default_locale[1] is None: lang = config.get('LANG') new_locale = lang - else: - new_locale = default_locale + else: new_locale = default_locale locale.setlocale(locale.LC_ALL, new_locale) reload(sys) sys.setdefaultencoding("UTF-8") @@ -395,7 +404,7 @@ def sub_path(directory,f): the paths. """ normalized = normpath(directory) - common = os.path.commonprefix([ directory, normpath(f) ]) + common = os.path.commonprefix([ normalized, normpath(f) ]) return common == normalized if __name__ == '__main__': diff --git a/python_apps/media-monitor2/tests/test_pure.py b/python_apps/media-monitor2/tests/test_pure.py index 02e753c8d..c3b08f874 100644 --- a/python_apps/media-monitor2/tests/test_pure.py +++ b/python_apps/media-monitor2/tests/test_pure.py @@ -60,4 +60,11 @@ class TestMMP(unittest.TestCase): self.assertRaises( ValueError, lambda : mmp.file_md5('/bull/shit/path') ) self.assertTrue( m1 == mmp.file_md5(p) ) + def test_sub_path(self): + f1 = "/home/testing/123.mp3" + d1 = "/home/testing" + d2 = "/home/testing/" + self.assertTrue( mmp.sub_path(d1, f1) ) + self.assertTrue( mmp.sub_path(d2, f1) ) + if __name__ == '__main__': unittest.main()