From 3b7e69c972a6c5ec844f1863e74e91ccb300f0c3 Mon Sep 17 00:00:00 2001 From: Levi Blackstone Date: Tue, 5 May 2015 07:43:00 -0500 Subject: [PATCH] Add PEP8 check and fix related issues - Add PEP8 section to tox.ini - Add hacking to requirements to enforce OpenStack style requirements - Fix formatting issues flagged by flake8 check - Add copyright notices to all remaining files - Update .gitignore file Change-Id: I7e9a0203ddf2002c08ac96800fe30c1c46ebba88 --- .gitignore | 6 ++++ requirements.txt | 1 + shoebox/archive.py | 4 +++ shoebox/disk_storage.py | 29 +++++++++++++----- shoebox/handlers.py | 6 +++- shoebox/roll_manager.py | 44 +++++++++++++++------------ test/integration/test_json_tarball.py | 36 +++++++++++++++------- test/integration/test_rolling.py | 35 +++++++++++++++------ test/test_callbacks.py | 19 ++++++++++-- test/test_disk_storage.py | 40 ++++++++++++++++-------- test/test_roll_checker.py | 17 ++++++++++- test/test_roll_manager.py | 34 +++++++++++++++------ tox.ini | 11 ++++++- 13 files changed, 206 insertions(+), 76 deletions(-) diff --git a/.gitignore b/.gitignore index ded6067..f637812 100644 --- a/.gitignore +++ b/.gitignore @@ -26,6 +26,7 @@ pip-log.txt .coverage .tox nosetests.xml +test_temp/* # Translations *.mo @@ -34,3 +35,8 @@ nosetests.xml .mr.developer.cfg .project .pydevproject + +# IDE Project Files +*.project +*.pydev* +*.idea diff --git a/requirements.txt b/requirements.txt index cc17005..70bf758 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,3 +1,4 @@ +hacking>=0.10.0,<0.11 notigen notification_utils python-dateutil diff --git a/shoebox/archive.py b/shoebox/archive.py index d95c37f..dd493f0 100644 --- a/shoebox/archive.py +++ b/shoebox/archive.py @@ -30,7 +30,9 @@ class Archive(object): class ArchiveWriter(Archive): """The active Archive for appending. + """ + def __init__(self, filename): super(ArchiveWriter, self).__init__(filename) self._open_file(filename) @@ -47,7 +49,9 @@ class ArchiveWriter(Archive): class ArchiveReader(Archive): """The active Archive for consuming. + """ + def __init__(self, filename): super(ArchiveReader, self).__init__(filename) self._open_file(filename) diff --git a/shoebox/disk_storage.py b/shoebox/disk_storage.py index 6234152..d8f21cc 100644 --- a/shoebox/disk_storage.py +++ b/shoebox/disk_storage.py @@ -1,6 +1,18 @@ -import calendar -import datetime -import json +# Copyright (c) 2014 Dark Secret Software Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + import struct @@ -61,7 +73,7 @@ class Version1(Version0): # *s = raw data # EXAMPLE - #-------- + # -------- # With above Event and Metadata # # Header schema: "iii" @@ -84,16 +96,16 @@ class Version1(Version0): self.header_size = struct.calcsize(self.header_schema) def _encode(self, s): - if isinstance(s, unicode): + if isinstance(s, unicode): return s.encode('utf-8') - return s + return s def pack(self, notification, metadata): nsize = len(notification) raw_block_schema = "i%ds" % nsize raw_block = struct.pack(raw_block_schema, nsize, notification) - metadata_items = ["i"] # appended with N "%ds"'s + metadata_items = ["i"] # appended with N "%ds"'s metadata_values = [len(metadata) * 4] # [n]=key, [n+1]=value for key, value in metadata.iteritems(): key = self._encode(key) @@ -142,7 +154,7 @@ class Version1(Version0): offset += struct.calcsize(lengths_schema) key_values = struct.unpack_from(key_value_schema, metadata_bytes, offset=offset) - metadata = dict((key_values[n], key_values[n+1]) + metadata = dict((key_values[n], key_values[n + 1]) for n in range(len(key_values))[::2]) raw = file_handle.read(header[1]) @@ -156,6 +168,7 @@ class Version1(Version0): VERSIONS = {1: Version1()} CURRENT_VERSION = 1 + def get_version_handler(version=CURRENT_VERSION): global VERSIONS diff --git a/shoebox/handlers.py b/shoebox/handlers.py index 477a51d..259ad05 100644 --- a/shoebox/handlers.py +++ b/shoebox/handlers.py @@ -34,6 +34,7 @@ class ArchiveCallback(object): def on_close(self, filename): """Called when an Archive is closed. + If you move/change the file/name return the new location so subsequent callbacks will have the right location. @@ -49,9 +50,11 @@ class CallbackList(ArchiveCallback): callback_str = self.config.get('callback_list', "") callback_str_list = [x.strip() for x in callback_str.split(",")] self.callbacks = [simport.load(c)(**self.config) - for c in callback_str_list] + for c in callback_str_list] + # TODO(Sandy): Need some exception handling around these. # The failure of one shouldn't stop processing. + def on_open(self, filename): for c in self.callbacks: c.on_open(filename) @@ -63,6 +66,7 @@ class CallbackList(ArchiveCallback): class ChangeExtensionCallback(ArchiveCallback): """filename.dat becomes filename.dat.done""" + def __init__(self, **kwargs): super(ChangeExtensionCallback, self).__init__(**kwargs) self.new_extension = kwargs.get('new_extension', '.done') diff --git a/shoebox/roll_manager.py b/shoebox/roll_manager.py index 9356a14..e621037 100644 --- a/shoebox/roll_manager.py +++ b/shoebox/roll_manager.py @@ -64,20 +64,20 @@ class RollManager(object): class ReadingRollManager(RollManager): def __init__(self, filename_template, directory=".", destination_directory=".", - archive_class = archive.ArchiveReader, + archive_class=archive.ArchiveReader, archive_callback=None, roll_size_mb=1000): super(ReadingRollManager, self).__init__( - filename_template, - directory=directory, - archive_callback=archive_callback, - archive_class=archive_class) + filename_template, + directory=directory, + archive_callback=archive_callback, + archive_class=archive_class) self.files_to_read = self._get_matching_files(directory, filename_template) def _get_matching_files(self, directory, filename_template): files = [os.path.join(directory, f) - for f in os.listdir(self.directory) - if os.path.isfile(os.path.join(directory, f))] + for f in os.listdir(self.directory) + if os.path.isfile(os.path.join(directory, f))] return sorted(fnmatch.filter(files, filename_template)) def read(self): @@ -107,15 +107,17 @@ class WritingRollManager(RollManager): archive_class=archive.ArchiveWriter, archive_callback=None, roll_size_mb=1000): super(WritingRollManager, self).__init__( - filename_template, - directory=directory, - archive_callback=archive_callback, - archive_class=archive_class) + filename_template, + directory=directory, + archive_callback=archive_callback, + archive_class=archive_class) self.roll_checker = roll_checker def write(self, metadata, payload): - """metadata is string:string dict. - payload must be encoded as string. + """Write metadata + + metadata is string:string dict. + payload must be encoded as string. """ a = self.get_active_archive() a.write(metadata, payload) @@ -143,11 +145,15 @@ class WritingRollManager(RollManager): class WritingJSONRollManager(object): - """No archiver. No roll checker. Just write 1 file line per json payload. - Once the file gets big enough, gzip the file and move - into the destination_directory. - Expects an external tool like rsync to move the file. - A SHA-256 of the payload may be included in the archive filename.""" + """Simple event archival. + + No archiver. No roll checker. Just write 1 file line per json payload. + Once the file gets big enough, gzip the file and move + into the destination_directory. + Expects an external tool like rsync to move the file. + A SHA-256 of the payload may be included in the archive filename. + """ + def __init__(self, *args, **kwargs): self.filename_template = args[0] self.directory = kwargs.get('directory', '.') @@ -193,7 +199,7 @@ class WritingJSONRollManager(object): self._get_time() >= (self.start_time + self.roll_after))) def _get_file_sha(self, filename): - block_size=2**20 + block_size = 2 ** 20 sha256 = hashlib.sha256() # no context manager, just to keep testing simple. f = open(filename, "r") diff --git a/test/integration/test_json_tarball.py b/test/integration/test_json_tarball.py index 83304ed..3ba3fa7 100644 --- a/test/integration/test_json_tarball.py +++ b/test/integration/test_json_tarball.py @@ -1,8 +1,21 @@ +# Copyright (c) 2014 Dark Secret Software Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + import datetime import gzip -import hashlib import json -import mock import os import shutil import unittest @@ -26,10 +39,10 @@ class TestDirectory(unittest.TestCase): def test_size_rolling(self): manager = roll_manager.WritingJSONRollManager( - "%Y_%m_%d_%X_%f_[[CRC]].event", - directory=TEMPDIR, - destination_directory=DESTDIR, - roll_size_mb=10) + "%Y_%m_%d_%X_%f_[[CRC]].event", + directory=TEMPDIR, + destination_directory=DESTDIR, + roll_size_mb=10) g = notigen.EventGenerator("test/integration/templates") entries = {} @@ -39,8 +52,9 @@ class TestDirectory(unittest.TestCase): if events: for event in events: metadata = {} - json_event = json.dumps(event, - cls=notification_utils.DateTimeEncoder) + json_event = json.dumps( + event, + cls=notification_utils.DateTimeEncoder) manager.write(metadata, json_event) msg_id = event['message_id'] entries[msg_id] = json_event @@ -49,7 +63,7 @@ class TestDirectory(unittest.TestCase): manager.close() manager._archive_working_files() - print "Starting entries:", len(entries) + print("Starting entries:", len(entries)) actual = len(entries) @@ -68,8 +82,8 @@ class TestDirectory(unittest.TestCase): num = len(file_content) - 1 total += num - print "In %s: %d of %d Remaining: %d" % (f, num, actual, - actual - total) + print("In %s: %d of %d Remaining: %d" + % (f, num, actual, actual - total)) if actual != total: raise Exception("Num generated != actual") diff --git a/test/integration/test_rolling.py b/test/integration/test_rolling.py index 4ad53be..8f860c3 100644 --- a/test/integration/test_rolling.py +++ b/test/integration/test_rolling.py @@ -1,6 +1,20 @@ +# Copyright (c) 2014 Dark Secret Software Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + import datetime import json -import mock import os import shutil import unittest @@ -8,7 +22,6 @@ import unittest import notification_utils import notigen -from shoebox import disk_storage from shoebox import roll_checker from shoebox import roll_manager @@ -64,14 +77,16 @@ class TestSizeRolling(unittest.TestCase): events = g.generate(now) if events: for event in events: - metadata = {'event': event['event_type'], - 'request_id': event['_context_request_id'], - 'generated': str(event['timestamp']), - 'uuid': event.get('payload', {} - ).get("instance_id", ""), - } - json_event = json.dumps(event, - cls=notification_utils.DateTimeEncoder) + metadata = { + 'event': event['event_type'], + 'request_id': event['_context_request_id'], + 'generated': str(event['timestamp']), + 'uuid': event.get('payload', {}).get( + "instance_id", ""), + } + json_event = json.dumps( + event, + cls=notification_utils.DateTimeEncoder) manager.write(metadata, json_event) entries.append((metadata, json_event)) diff --git a/test/test_callbacks.py b/test/test_callbacks.py index 8b7e3eb..531479b 100644 --- a/test/test_callbacks.py +++ b/test/test_callbacks.py @@ -1,5 +1,19 @@ -import unittest +# Copyright (c) 2014 Dark Secret Software Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import unittest from shoebox import handlers @@ -24,6 +38,5 @@ class TestCallbackList(unittest.TestCase): # (the 'test' module). self.assertTrue("FooCallback" in str(type(c.callbacks[0]))) self.assertTrue(isinstance(c.callbacks[1], - handlers.ChangeExtensionCallback)) + handlers.ChangeExtensionCallback)) self.assertTrue(isinstance(c.callbacks[2], BlahCallback)) - diff --git a/test/test_disk_storage.py b/test/test_disk_storage.py index 5083f15..89d6ee8 100644 --- a/test/test_disk_storage.py +++ b/test/test_disk_storage.py @@ -1,20 +1,32 @@ -import datetime -import mock +# Copyright (c) 2014 Dark Secret Software Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + import json +import mock import struct import unittest -import dateutil.tz - from shoebox import disk_storage class TestVersion0(unittest.TestCase): def setUp(self): - self.v0 = disk_storage.Version0() + self.v0 = disk_storage.Version0() def test_make_preamble(self): - self.assertEqual(6, len(self.v0.make_preamble(99))) + self.assertEqual(6, len(self.v0.make_preamble(99))) def test_load_preamble_bad_bor(self): file_handle = mock.Mock() @@ -24,14 +36,16 @@ class TestVersion0(unittest.TestCase): def test_load_preamble(self): file_handle = mock.Mock() - file_handle.read.return_value = struct.pack("ih", - disk_storage.BOR_MAGIC_NUMBER, 99) + file_handle.read.return_value = struct.pack( + "ih", + disk_storage.BOR_MAGIC_NUMBER, + 99) self.assertEqual(99, self.v0.load_preamble(file_handle)) class TestVersion1(unittest.TestCase): def setUp(self): - self.v1 = disk_storage.Version1() + self.v1 = disk_storage.Version1() def test_no_metadata(self): metadata = {} @@ -76,7 +90,7 @@ class TestVersion1(unittest.TestCase): blocks = blocks[1:] # Remove preamble # break the EOR marker - print len(blocks[0]) + print(len(blocks[0])) newblock = blocks[0][:8] + '\x00\x00\x01\x02' blocks = list(blocks) blocks[0] = newblock @@ -110,8 +124,10 @@ class TestHelpers(unittest.TestCase): def test_unpack_notification(self): file_handle = mock.Mock() - file_handle.read.return_value = struct.pack("ih", - disk_storage.BOR_MAGIC_NUMBER, 99) + file_handle.read.return_value = struct.pack( + "ih", + disk_storage.BOR_MAGIC_NUMBER, + 99) with mock.patch('shoebox.disk_storage.get_version_handler') as h: fake_handler = mock.Mock() diff --git a/test/test_roll_checker.py b/test/test_roll_checker.py index 15cbb31..2ea793c 100644 --- a/test/test_roll_checker.py +++ b/test/test_roll_checker.py @@ -1,3 +1,18 @@ +# Copyright (c) 2014 Dark Secret Software Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + import datetime import mock import unittest @@ -33,7 +48,7 @@ class TestRollChecker(unittest.TestCase): self.assertFalse(x.check(None)) with mock.patch.object(notification_utils, 'now') as dt: - dt.return_value = now + one_hour - datetime.timedelta(seconds = 1) + dt.return_value = now + one_hour - datetime.timedelta(seconds=1) self.assertFalse(x.check(None)) def test_size_roll_checker_end(self): diff --git a/test/test_roll_manager.py b/test/test_roll_manager.py index 2db62d3..56b9a76 100644 --- a/test/test_roll_manager.py +++ b/test/test_roll_manager.py @@ -1,10 +1,23 @@ +# Copyright (c) 2014 Dark Secret Software Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + import datetime -import mock -import os import unittest +import mock import notification_utils - from shoebox import archive from shoebox import roll_checker from shoebox import roll_manager @@ -53,7 +66,7 @@ class TestWritingRollManager(unittest.TestCase): def test_correct_archiver(self): x = roll_manager.WritingRollManager("foo", None) - print x.archive_class + print(x.archive_class) self.assertEqual(x.archive_class, archive.ArchiveWriter) def test_get_active_archive(self): @@ -63,8 +76,8 @@ class TestWritingRollManager(unittest.TestCase): x = roll_manager.WritingRollManager(filename_template, checker, archive_callback=callback, archive_class=FakeArchive) - with mock.patch("shoebox.archive.ArchiveWriter._open_file") as of: - arc = x.get_active_archive() + with mock.patch("shoebox.archive.ArchiveWriter._open_file"): + x.get_active_archive() self.assertTrue(checker.start.called) self.assertTrue(callback.on_open.called) @@ -105,7 +118,7 @@ class TestJSONRollManager(unittest.TestCase): td.return_value = 123.45 dt.return_value = now x = roll_manager.WritingJSONRollManager( - "%Y%m%d [[TIMESTAMP]] [[CRC]].foo") + "%Y%m%d [[TIMESTAMP]] [[CRC]].foo") fn = x._make_filename("mycrc", "foo") self.assertEqual("foo/20140201_123.45_mycrc.foo", fn) @@ -124,10 +137,11 @@ class TestJSONRollManager(unittest.TestCase): def test_should_roll(self, awf): rm = roll_manager.WritingJSONRollManager("template.foo") rm.roll_size_mb = 10 - self.assertFalse(rm._should_roll(9*1048576)) - self.assertTrue(rm._should_roll(10*1048576)) + self.assertFalse(rm._should_roll(9 * 1048576)) + self.assertTrue(rm._should_roll(10 * 1048576)) - rm = roll_manager.WritingJSONRollManager("template.foo", roll_minutes=10) + rm = roll_manager.WritingJSONRollManager("template.foo", + roll_minutes=10) self.assertFalse(rm._should_roll(0)) self.assertFalse(rm._should_roll(1)) with mock.patch.object(rm, "_get_time") as gt: diff --git a/tox.ini b/tox.ini index 8e4c1a8..1520e68 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py26, py27 +envlist = py26, py27, pep8 [testenv] deps = @@ -12,3 +12,12 @@ deps = simport commands = nosetests -d -v --with-coverage --cover-inclusive --cover-package shoebox [] + +[testenv:pep8] +commands = + flake8 + +[flake8] +ignore = +exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg +show-source = True