From a4655432604a758f2725c5badc120f4836ffcbda Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 23 Oct 2018 15:19:09 +0200 Subject: [PATCH] Drop threading lock and global try/except The Lock wasn't actually preventing anything, and I don't think anything would need ot be protected here. mkdtemp should be thread-safe. We also remove the try/except Statement. I think it'd be better to expose any exceptions here instead of producing broken capsules and logging an exception. --- lib/tool_shed/capsule/capsule_manager.py | 79 +++++++++++------------- 1 file changed, 35 insertions(+), 44 deletions(-) diff --git a/lib/tool_shed/capsule/capsule_manager.py b/lib/tool_shed/capsule/capsule_manager.py index d27d4e2f141..46b7d1bffeb 100644 --- a/lib/tool_shed/capsule/capsule_manager.py +++ b/lib/tool_shed/capsule/capsule_manager.py @@ -4,7 +4,6 @@ import os import shutil import tarfile import tempfile -import threading from time import gmtime, strftime import requests @@ -66,52 +65,44 @@ class ExportRepositoryManager(object): ordered_repository_ids = [self.repository_id] ordered_repositories = [self.repository] ordered_changeset_revisions = [repository_metadata.changeset_revision] - repositories_archive = None error_messages = '' - lock = threading.Lock() - lock.acquire(True) + repositories_archive = tarfile.open(repositories_archive_filename, "w:%s" % self.file_type) + exported_repository_registry = ExportedRepositoryRegistry() + for repository_id, ordered_repository, ordered_changeset_revision in zip(ordered_repository_ids, + ordered_repositories, + ordered_changeset_revisions): + with self.__tempdir(prefix='tmp-toolshed-export-er') as work_dir: + repository_archive, error_message = self.generate_repository_archive(ordered_repository, + ordered_changeset_revision, + work_dir) + if error_message: + error_messages = '%s %s' % (error_messages, error_message) + else: + archive_name = str(os.path.basename(repository_archive.name)) + repositories_archive.add(repository_archive.name, arcname=archive_name) + attributes, sub_elements = self.get_repository_attributes_and_sub_elements(ordered_repository, + archive_name) + elem = xml_util.create_element('repository', attributes=attributes, sub_elements=sub_elements) + exported_repository_registry.exported_repository_elems.append(elem) + # Keep information about the export in a file named export_info.xml in the archive. + sub_elements = self.generate_export_elem() + export_elem = xml_util.create_element('export_info', attributes=None, sub_elements=sub_elements) + tmp_export_info = xml_util.create_and_write_tmp_file(export_elem) try: - repositories_archive = tarfile.open(repositories_archive_filename, "w:%s" % self.file_type) - exported_repository_registry = ExportedRepositoryRegistry() - for repository_id, ordered_repository, ordered_changeset_revision in zip(ordered_repository_ids, - ordered_repositories, - ordered_changeset_revisions): - with self.__tempdir(prefix='tmp-toolshed-export-er') as work_dir: - repository_archive, error_message = self.generate_repository_archive(ordered_repository, - ordered_changeset_revision, - work_dir) - if error_message: - error_messages = '%s %s' % (error_messages, error_message) - else: - archive_name = str(os.path.basename(repository_archive.name)) - repositories_archive.add(repository_archive.name, arcname=archive_name) - attributes, sub_elements = self.get_repository_attributes_and_sub_elements(ordered_repository, - archive_name) - elem = xml_util.create_element('repository', attributes=attributes, sub_elements=sub_elements) - exported_repository_registry.exported_repository_elems.append(elem) - # Keep information about the export in a file named export_info.xml in the archive. - sub_elements = self.generate_export_elem() - export_elem = xml_util.create_element('export_info', attributes=None, sub_elements=sub_elements) - tmp_export_info = xml_util.create_and_write_tmp_file(export_elem) - try: - repositories_archive.add(tmp_export_info, arcname='export_info.xml') - finally: - if os.path.exists(tmp_export_info): - os.remove(tmp_export_info) - # Write the manifest, which must preserve the order in which the repositories should be imported. - exported_repository_root = xml_util.create_element('repositories') - for exported_repository_elem in exported_repository_registry.exported_repository_elems: - exported_repository_root.append(exported_repository_elem) - tmp_manifest = xml_util.create_and_write_tmp_file(exported_repository_root) - try: - repositories_archive.add(tmp_manifest, arcname='manifest.xml') - finally: - if os.path.exists(tmp_manifest): - os.remove(tmp_manifest) - except Exception as e: - log.exception(str(e)) + repositories_archive.add(tmp_export_info, arcname='export_info.xml') finally: - lock.release() + if os.path.exists(tmp_export_info): + os.remove(tmp_export_info) + # Write the manifest, which must preserve the order in which the repositories should be imported. + exported_repository_root = xml_util.create_element('repositories', attributes=None, sub_elements=None) + for elem in exported_repository_registry.exported_repository_elems: + exported_repository_root.append(elem) + tmp_manifest = xml_util.create_and_write_tmp_file(exported_repository_root) + try: + repositories_archive.add(tmp_manifest, arcname='manifest.xml') + finally: + if os.path.exists(tmp_manifest): + os.remove(tmp_manifest) if repositories_archive is not None: repositories_archive.close() if self.using_api: