commit 45463c5a083965799b3500ed2715a43baf89b3aa Author: Damian Johnson <atagar@torproject.org> Date: Sat Jan 4 15:21:59 2020 -0800 Open tarfile using 'with' statement Addressing one of our TODO comments. This one's kinda moot since we're dropping this whole module but... meh. --- stem/descriptor/__init__.py | 8 +---- stem/descriptor/reader.py | 56 +++++++++++++------------------ test/integ/installation.py | 4 +-- test/unit/descriptor/reader.py | 12 +------ test/unit/descriptor/server_descriptor.py | 14 +++----- 5 files changed, 32 insertions(+), 62 deletions(-) diff --git a/stem/descriptor/__init__.py b/stem/descriptor/__init__.py index ec299289..b7270ac2 100644 --- a/stem/descriptor/__init__.py +++ b/stem/descriptor/__init__.py @@ -447,16 +447,10 @@ def _parse_file_for_path(descriptor_file, *args, **kwargs): def _parse_file_for_tar_path(descriptor_file, *args, **kwargs): - # TODO: use 'with' for tarfile after dropping python 2.6 support - tar_file = tarfile.open(descriptor_file) - - try: + with tarfile.open(descriptor_file) as tar_file: for desc in parse_file(tar_file, *args, **kwargs): desc._set_path(os.path.abspath(descriptor_file)) yield desc - finally: - if tar_file: - tar_file.close() def _parse_file_for_tarfile(descriptor_file, *args, **kwargs): diff --git a/stem/descriptor/reader.py b/stem/descriptor/reader.py index 8ef6f22c..e75cdb7e 100644 --- a/stem/descriptor/reader.py +++ b/stem/descriptor/reader.py @@ -255,7 +255,7 @@ class DescriptorReader(object): :param bool validate: checks the validity of the descriptor's content if **True**, skips these checks otherwise :param bool follow_links: determines if we'll follow symlinks when traversing - directories (requires python 2.6) + directories :param int buffer_size: descriptors we'll buffer before waiting for some to be read, this is unbounded if zero :param str persistence_path: if set we will load and save processed file @@ -521,41 +521,31 @@ class DescriptorReader(object): self._notify_skip_listeners(target, ReadFailed(exc)) def _handle_archive(self, target): - # TODO: When dropping python 2.6 support go back to using 'with' for - # tarfiles... - # - # http://bugs.python.org/issue7232 - - tar_file = None - try: - self._notify_read_listeners(target) - tar_file = tarfile.open(target) - - for tar_entry in tar_file: - if tar_entry.isfile(): - entry = tar_file.extractfile(tar_entry) - - try: - for desc in stem.descriptor.parse_file(entry, validate = self._validate, document_handler = self._document_handler, **self._kwargs): - if self._is_stopped.is_set(): - return - - desc._set_path(os.path.abspath(target)) - desc._set_archive_path(tar_entry.name) - self._unreturned_descriptors.put(desc) - self._iter_notice.set() - except TypeError as exc: - self._notify_skip_listeners(target, ParsingFailure(exc)) - except ValueError as exc: - self._notify_skip_listeners(target, ParsingFailure(exc)) - finally: - entry.close() + with tarfile.open(target) as tar_file: + self._notify_read_listeners(target) + + for tar_entry in tar_file: + if tar_entry.isfile(): + entry = tar_file.extractfile(tar_entry) + + try: + for desc in stem.descriptor.parse_file(entry, validate = self._validate, document_handler = self._document_handler, **self._kwargs): + if self._is_stopped.is_set(): + return + + desc._set_path(os.path.abspath(target)) + desc._set_archive_path(tar_entry.name) + self._unreturned_descriptors.put(desc) + self._iter_notice.set() + except TypeError as exc: + self._notify_skip_listeners(target, ParsingFailure(exc)) + except ValueError as exc: + self._notify_skip_listeners(target, ParsingFailure(exc)) + finally: + entry.close() except IOError as exc: self._notify_skip_listeners(target, ReadFailed(exc)) - finally: - if tar_file: - tar_file.close() def _notify_read_listeners(self, path): for listener in self._read_listeners: diff --git a/test/integ/installation.py b/test/integ/installation.py index 627f0314..2f41ef58 100644 --- a/test/integ/installation.py +++ b/test/integ/installation.py @@ -127,8 +127,8 @@ class TestInstallation(unittest.TestCase): # tarball has a prefix 'stem-[verion]' directory so stipping that out - dist_tar = tarfile.open(os.path.join(DIST_PATH, 'stem-dry-run-%s.tar.gz' % stem.__version__)) - tar_contents = ['/'.join(info.name.split('/')[1:]) for info in dist_tar.getmembers() if info.isfile()] + with tarfile.open(os.path.join(DIST_PATH, 'stem-dry-run-%s.tar.gz' % stem.__version__)) as dist_tar: + tar_contents = ['/'.join(info.name.split('/')[1:]) for info in dist_tar.getmembers() if info.isfile()] issues = [] diff --git a/test/unit/descriptor/reader.py b/test/unit/descriptor/reader.py index 39a5d669..f49183e5 100644 --- a/test/unit/descriptor/reader.py +++ b/test/unit/descriptor/reader.py @@ -40,23 +40,13 @@ def _get_raw_tar_descriptors(): test_path = os.path.join(DESCRIPTOR_TEST_DATA, 'descriptor_archive.tar') raw_descriptors = [] - # TODO: revert to using the 'with' keyword for this when dropping python - # 2.6 support - - tar_file = None - - try: - tar_file = tarfile.open(test_path) - + with tarfile.open(test_path) as tar_file: for tar_entry in tar_file: if tar_entry.isfile(): entry = tar_file.extractfile(tar_entry) entry.readline() # strip header raw_descriptors.append(entry.read().decode('utf-8', 'replace')) entry.close() - finally: - if tar_file: - tar_file.close() TAR_DESCRIPTORS = raw_descriptors diff --git a/test/unit/descriptor/server_descriptor.py b/test/unit/descriptor/server_descriptor.py index 2f448404..aeb58df1 100644 --- a/test/unit/descriptor/server_descriptor.py +++ b/test/unit/descriptor/server_descriptor.py @@ -65,16 +65,12 @@ class TestServerDescriptor(unittest.TestCase): Fetch server descriptors via parse_file() for a tarfile object. """ - # TODO: When dropping python 2.6 support we can go back to using the 'with' - # keyword here. + with tarfile.open(get_resource('descriptor_archive.tar')) as tar_file: + descriptors = list(stem.descriptor.parse_file(tar_file)) + self.assertEqual(3, len(descriptors)) - tar_file = tarfile.open(get_resource('descriptor_archive.tar')) - descriptors = list(stem.descriptor.parse_file(tar_file)) - self.assertEqual(3, len(descriptors)) - - fingerprints = set([desc.fingerprint for desc in descriptors]) - self.assertEqual(TARFILE_FINGERPRINTS, fingerprints) - tar_file.close() + fingerprints = set([desc.fingerprint for desc in descriptors]) + self.assertEqual(TARFILE_FINGERPRINTS, fingerprints) def test_metrics_descriptor(self): """