Skip to content

Commit

Permalink
Fix more instances of issue #21 - use copyfile instead of move
Browse files Browse the repository at this point in the history
  • Loading branch information
jfischer committed May 5, 2019
1 parent c30a2c5 commit 8adcf14
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 12 deletions.
2 changes: 1 addition & 1 deletion dataworkspaces/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
# Copyright 2018,2019 by MPI-SWS and Data-ken Research. Licensed under Apache 2.0. See LICENSE.txt.

__version__ = '1.0.2'
__version__ = '1.0.3'
17 changes: 12 additions & 5 deletions dataworkspaces/resources/git_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from os.path import realpath, basename, isdir, join, dirname, exists
import stat
import click
import shutil

from dataworkspaces.errors import ConfigurationError, InternalError
from dataworkspaces.utils.subprocess_utils import \
Expand Down Expand Up @@ -137,7 +138,8 @@ def results_move_current_files(self, rel_dest_root, exclude_files,

def add_results_file(self, temp_path, rel_dest_path):
"""Copy a results file from the temporary location to
the specified path in the resource.
the specified path in the resource. Caller responsible for
cleanup of temp_path.
"""
assert exists(temp_path)
assert self.role==ResourceRoles.RESULTS
Expand All @@ -146,7 +148,9 @@ def add_results_file(self, temp_path, rel_dest_path):
parent_dir = dirname(abs_dest_path)
if not exists(parent_dir):
os.makedirs(parent_dir)
os.rename(temp_path, abs_dest_path)
# Need to use copy instead of move, because /tmp might be in a separate
# filesystem (see issue #21). Caller will do cleanup of temp file.
shutil.copyfile(temp_path, abs_dest_path)
call_subprocess([GIT_EXE_PATH, 'add', rel_dest_path],
cwd=self.local_path, verbose=self.verbose)
call_subprocess([GIT_EXE_PATH, 'commit',
Expand Down Expand Up @@ -425,14 +429,17 @@ def results_move_current_files(self, rel_dest_root, exclude_files,

def add_results_file(self, temp_path, rel_dest_path):
"""Move a results file from the temporary location to
the specified path in the resource.
the specified path in the resource. Caller responsible
for cleanup of temp_path
"""
assert exists(temp_path)
abs_dest_path = join(self.local_path, rel_dest_path)
parent_dir = dirname(abs_dest_path)
if not exists(parent_dir):
os.makedirs(parent_dir)
os.rename(temp_path, abs_dest_path)
# Need to use copy instead of move, because /tmp might be in a separate
# filesystem (see issue #21). Caller will do cleanup of temp file.
shutil.copyfile(temp_path, abs_dest_path)
rel_path_in_repo = join(self.relative_path, rel_dest_path)
call_subprocess([GIT_EXE_PATH, 'add', rel_path_in_repo],
cwd=self.workspace_dir, verbose=self.verbose)
Expand Down Expand Up @@ -529,7 +536,7 @@ def results_move_current_files(self, rel_dest_root, exclude_files,

def add_results_file(self, temp_path, rel_dest_path):
"""Copy a results file from the temporary location to
the specified path in the resource.
the specified path in the resource. Caller responsible for cleanup.
"""
raise InternalError("add_results_file should not be called for %s" %
self.__class__.__name__)
Expand Down
8 changes: 5 additions & 3 deletions dataworkspaces/resources/local_file_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
"""
from errno import EEXIST
import os
import os.path
import os.path
import shutil

from dataworkspaces.errors import ConfigurationError
from dataworkspaces.utils.subprocess_utils import call_subprocess
Expand Down Expand Up @@ -85,15 +86,16 @@ def snapshot(self):

def add_results_file(self, temp_path, rel_dest_path):
"""Move a results file from the temporary location to
the specified path in the resource.
the specified path in the resource. Caller responsible
for cleanup.
"""
assert self.role==ResourceRoles.RESULTS
assert os.path.exists(temp_path)
abs_dest_path = os.path.join(self.local_path, rel_dest_path)
parent_dir = os.path.dirname(abs_dest_path)
if not os.path.isdir(parent_dir):
os.makedirs(parent_dir)
os.rename(temp_path, abs_dest_path)
shutil.copyfile(temp_path, abs_dest_path)

def restore_prechecks(self, hashval):
print("IN RESTORE")
Expand Down
6 changes: 4 additions & 2 deletions dataworkspaces/resources/rclone_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import os
import os.path
import stat
import shutil

from dataworkspaces.errors import ConfigurationError
from dataworkspaces.utils.subprocess_utils import call_subprocess
Expand Down Expand Up @@ -123,15 +124,16 @@ def snapshot(self):

def add_results_file(self, temp_path, rel_dest_path):
"""Move a results file from the temporary location to
the specified path in the resource.
the specified path in the resource. Caller responsible
for cleanup.
"""
assert self.role==ResourceRoles.RESULTS
assert os.path.exists(temp_path)
abs_dest_path = os.path.join(self.local_path, rel_dest_path)
parent_dir = os.path.dirname(abs_dest_path)
if not os.path.isdir(parent_dir):
os.makedirs(parent_dir)
os.rename(temp_path, abs_dest_path)
shutil.copy(temp_path, abs_dest_path)

def restore_prechecks(self, hashval):
pass
Expand Down
3 changes: 2 additions & 1 deletion dataworkspaces/resources/resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,8 @@ def results_move_current_files(self, rel_dest_root, exclude_files,

def add_results_file(self, temp_path, rel_dest_path):
"""Move a results file from the temporary location to
the specified path in the resource.
the specified path in the resource. Caller responsible for
cleanup of temp_path.
"""
raise NotImplementedError(self.__class__.__name__)

Expand Down

0 comments on commit 8adcf14

Please sign in to comment.