From c764dc20c641bb6ef58df1c4b29f7490b6417276 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Tue, 6 Dec 2022 16:00:49 +0100 Subject: [PATCH 1/6] normalize paths when added to queue --- openpype/lib/file_transaction.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openpype/lib/file_transaction.py b/openpype/lib/file_transaction.py index 1626bec6b6..ce7ef100c1 100644 --- a/openpype/lib/file_transaction.py +++ b/openpype/lib/file_transaction.py @@ -66,8 +66,8 @@ class FileTransaction(object): """Add a new file to transfer queue""" opts = {"mode": mode} - src = os.path.abspath(src) - dst = os.path.abspath(dst) + src = os.path.normpath(os.path.abspath(src)) + dst = os.path.normpath(os.path.abspath(dst)) if dst in self._transfers: queued_src = self._transfers[dst][0] From 18a9c5568426f6b67dc23d90742c6ac140e38800 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Tue, 6 Dec 2022 16:02:21 +0100 Subject: [PATCH 2/6] skip if source and destination are the same paths --- openpype/lib/file_transaction.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/openpype/lib/file_transaction.py b/openpype/lib/file_transaction.py index ce7ef100c1..4aedc62fb6 100644 --- a/openpype/lib/file_transaction.py +++ b/openpype/lib/file_transaction.py @@ -84,9 +84,11 @@ class FileTransaction(object): self._transfers[dst] = (src, opts) def process(self): - # Backup any existing files - for dst in self._transfers.keys(): + for dst, (src, opts) in self._transfers.items(): + if not os.path.isdir(src) and dst == src: + continue + if os.path.exists(dst): # Backup original file # todo: add timestamp or uuid to ensure unique @@ -98,6 +100,12 @@ class FileTransaction(object): # Copy the files to transfer for dst, (src, opts) in self._transfers.items(): + if not os.path.isdir(src) and dst == src: + self.log.debug( + "Source and destionation are same files {} -> {}".format( + src, dst)) + continue + self._create_folder_for_file(dst) if opts["mode"] == self.MODE_COPY: From 36dcab11c1c54cec6040456de8ec74ee20635111 Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Tue, 6 Dec 2022 16:02:52 +0100 Subject: [PATCH 3/6] formatting changes --- openpype/lib/file_transaction.py | 60 ++++++++++++++++++-------------- 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/openpype/lib/file_transaction.py b/openpype/lib/file_transaction.py index 4aedc62fb6..2d706adaef 100644 --- a/openpype/lib/file_transaction.py +++ b/openpype/lib/file_transaction.py @@ -14,9 +14,9 @@ else: class FileTransaction(object): - """ + """File transaction with rollback options. - The file transaction is a three step process. + The file transaction is a three-step process. 1) Rename any existing files to a "temporary backup" during `process()` 2) Copy the files to final destination during `process()` @@ -39,14 +39,12 @@ class FileTransaction(object): Warning: Any folders created during the transfer will not be removed. - """ MODE_COPY = 0 MODE_HARDLINK = 1 def __init__(self, log=None): - if log is None: log = logging.getLogger("FileTransaction") @@ -63,7 +61,14 @@ class FileTransaction(object): self._backup_to_original = {} def add(self, src, dst, mode=MODE_COPY): - """Add a new file to transfer queue""" + """Add a new file to transfer queue. + + Args: + src (str): Source path. + dst (str): Destination path. + mode (MODE_COPY, MODE_HARDLINK): Transfer mode. + """ + opts = {"mode": mode} src = os.path.normpath(os.path.abspath(src)) @@ -72,14 +77,15 @@ class FileTransaction(object): if dst in self._transfers: queued_src = self._transfers[dst][0] if src == queued_src: - self.log.debug("File transfer was already " - "in queue: {} -> {}".format(src, dst)) + self.log.debug( + "File transfer was already in queue: {} -> {}".format( + src, dst)) return else: self.log.warning("File transfer in queue replaced..") - self.log.debug("Removed from queue: " - "{} -> {}".format(queued_src, dst)) - self.log.debug("Added to queue: {} -> {}".format(src, dst)) + self.log.debug( + "Removed from queue: {} -> {} replaced by {} -> {}".format( + queued_src, dst, src, dst)) self._transfers[dst] = (src, opts) @@ -94,8 +100,8 @@ class FileTransaction(object): # todo: add timestamp or uuid to ensure unique backup = dst + ".bak" self._backup_to_original[backup] = dst - self.log.debug("Backup existing file: " - "{} -> {}".format(dst, backup)) + self.log.debug( + "Backup existing file: {} -> {}".format(dst, backup)) os.rename(dst, backup) # Copy the files to transfer @@ -112,8 +118,8 @@ class FileTransaction(object): self.log.debug("Copying file ... {} -> {}".format(src, dst)) copyfile(src, dst) elif opts["mode"] == self.MODE_HARDLINK: - self.log.debug("Hardlinking file ... {} -> {}".format(src, - dst)) + self.log.debug("Hardlinking file ... {} -> {}".format( + src, dst)) create_hard_link(src, dst) self._transferred.append(dst) @@ -124,23 +130,21 @@ class FileTransaction(object): try: os.remove(backup) except OSError: - self.log.error("Failed to remove backup file: " - "{}".format(backup), - exc_info=True) + self.log.error( + "Failed to remove backup file: {}".format(backup), + exc_info=True) def rollback(self): - errors = 0 - # Rollback any transferred files for path in self._transferred: try: os.remove(path) except OSError: errors += 1 - self.log.error("Failed to rollback created file: " - "{}".format(path), - exc_info=True) + self.log.error( + "Failed to rollback created file: {}".format(path), + exc_info=True) # Rollback the backups for backup, original in self._backup_to_original.items(): @@ -148,13 +152,15 @@ class FileTransaction(object): os.rename(backup, original) except OSError: errors += 1 - self.log.error("Failed to restore original file: " - "{} -> {}".format(backup, original), - exc_info=True) + self.log.error( + "Failed to restore original file: {} -> {}".format( + backup, original), + exc_info=True) if errors: - self.log.error("{} errors occurred during " - "rollback.".format(errors), exc_info=True) + self.log.error( + "{} errors occurred during rollback.".format(errors), + exc_info=True) six.reraise(*sys.exc_info()) @property From ee71a051b6066011fc4cfe8cd261de8fe9081fad Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Tue, 6 Dec 2022 16:05:14 +0100 Subject: [PATCH 4/6] removed redundant check of directory --- openpype/lib/file_transaction.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openpype/lib/file_transaction.py b/openpype/lib/file_transaction.py index 2d706adaef..6f285d73a8 100644 --- a/openpype/lib/file_transaction.py +++ b/openpype/lib/file_transaction.py @@ -92,7 +92,7 @@ class FileTransaction(object): def process(self): # Backup any existing files for dst, (src, opts) in self._transfers.items(): - if not os.path.isdir(src) and dst == src: + if dst == src: continue if os.path.exists(dst): @@ -106,7 +106,7 @@ class FileTransaction(object): # Copy the files to transfer for dst, (src, opts) in self._transfers.items(): - if not os.path.isdir(src) and dst == src: + if dst == src: self.log.debug( "Source and destionation are same files {} -> {}".format( src, dst)) From 9f2cd89e1521bca7af39927d09655867a082456f Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Tue, 6 Dec 2022 16:06:52 +0100 Subject: [PATCH 5/6] remove unused variable --- openpype/lib/file_transaction.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/openpype/lib/file_transaction.py b/openpype/lib/file_transaction.py index 6f285d73a8..f265b8815c 100644 --- a/openpype/lib/file_transaction.py +++ b/openpype/lib/file_transaction.py @@ -91,18 +91,17 @@ class FileTransaction(object): def process(self): # Backup any existing files - for dst, (src, opts) in self._transfers.items(): - if dst == src: + for dst, (src, _) in self._transfers.items(): + if dst == src or not os.path.exists(dst): continue - if os.path.exists(dst): - # Backup original file - # todo: add timestamp or uuid to ensure unique - backup = dst + ".bak" - self._backup_to_original[backup] = dst - self.log.debug( - "Backup existing file: {} -> {}".format(dst, backup)) - os.rename(dst, backup) + # Backup original file + # todo: add timestamp or uuid to ensure unique + backup = dst + ".bak" + self._backup_to_original[backup] = dst + self.log.debug( + "Backup existing file: {} -> {}".format(dst, backup)) + os.rename(dst, backup) # Copy the files to transfer for dst, (src, opts) in self._transfers.items(): From 2c55ee55c266dbfe90394918e604ed87c51d619e Mon Sep 17 00:00:00 2001 From: Jakub Trllo Date: Tue, 6 Dec 2022 16:09:54 +0100 Subject: [PATCH 6/6] remove source and destination check from integrate --- openpype/plugins/publish/integrate.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/openpype/plugins/publish/integrate.py b/openpype/plugins/publish/integrate.py index 57a642c635..6a85a87129 100644 --- a/openpype/plugins/publish/integrate.py +++ b/openpype/plugins/publish/integrate.py @@ -291,9 +291,6 @@ class IntegrateAsset(pyblish.api.InstancePlugin): instance) for src, dst in prepared["transfers"]: - if src == dst: - continue - # todo: add support for hardlink transfers file_transactions.add(src, dst)