From 283f84882209e67d0c989ab02825a7dec5ab2843 Mon Sep 17 00:00:00 2001 From: "Mr. Outis" Date: Fri, 1 Nov 2019 13:17:22 -0600 Subject: [PATCH] sftp: make move an atomic operation --- dvc/remote/ssh/connection.py | 17 +++++++++-------- tests/unit/remote/ssh/test_connection.py | 6 ++++++ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/dvc/remote/ssh/connection.py b/dvc/remote/ssh/connection.py index 657a059ec..f8a38a0eb 100644 --- a/dvc/remote/ssh/connection.py +++ b/dvc/remote/ssh/connection.py @@ -185,17 +185,18 @@ class SSHConnection: self.sftp.get(src, dest, callback=pbar.update_to) def move(self, src, dst): - """Use `SFTP RENAME` operation when possible, otherwise, - fallback to copy and remove, since the former one doesn't - work across drives. + """Atomically move src to dst. + + Moving is performed in two stages to make the whole operation atomic in + case src and dst are on different filesystems and actual physical + copying of data is happening. """ self.makedirs(posixpath.dirname(dst)) - try: - self.sftp.rename(src, dst) - except OSError: - self.copy(src, dst) - self.remove(src) + tmp = tmp_fname(dst) + self.copy(src, tmp) + self.sftp.rename(tmp, dst) + self.remove(src) def upload(self, src, dest, no_progress_bar=False, progress_title=None): self.makedirs(posixpath.dirname(dest)) diff --git a/tests/unit/remote/ssh/test_connection.py b/tests/unit/remote/ssh/test_connection.py index f0c12ec21..6b1300ce3 100644 --- a/tests/unit/remote/ssh/test_connection.py +++ b/tests/unit/remote/ssh/test_connection.py @@ -111,3 +111,9 @@ def test_hardlink(repo_dir, ssh): def test_copy(repo_dir, ssh): ssh.copy("foo", "link") assert filecmp.cmp("foo", "link") + + +def test_move(repo_dir, ssh): + ssh.move("foo", "copy") + assert os.path.exists("copy") + assert not os.path.exists("foo")