Skip to content

Commit 1ceaecb

Browse files
committed
Reorganize file.transfer_from to move some logic earlier
Do not download directly to destination directory if the destination directory does not exist.
1 parent d4ad5c3 commit 1ceaecb

File tree

1 file changed

+45
-41
lines changed

1 file changed

+45
-41
lines changed

python/lsst/resources/file.py

+45-41
Original file line numberDiff line numberDiff line change
@@ -182,10 +182,53 @@ def transfer_from(
182182
transfer,
183183
)
184184

185+
# The output location should not exist unless overwrite=True.
186+
# Rather than use `exists()`, use os.stat since we might need
187+
# the full answer later.
188+
dest_stat: os.stat_result | None
189+
try:
190+
# Do not read through links of the file itself.
191+
dest_stat = os.lstat(self.ospath)
192+
except FileNotFoundError:
193+
dest_stat = None
194+
195+
# It is possible that the source URI and target URI refer
196+
# to the same file. This can happen for a number of reasons
197+
# (such as soft links in the path, or they really are the same).
198+
# In that case log a message and return as if the transfer
199+
# completed (it technically did). A temporary file download
200+
# can't be the same so the test can be skipped.
201+
if dest_stat and src.isLocal and not src.isTemporary:
202+
# Be consistent and use lstat here (even though realpath
203+
# has been called). It does not harm.
204+
local_src_stat = os.lstat(src.ospath)
205+
if dest_stat.st_ino == local_src_stat.st_ino and dest_stat.st_dev == local_src_stat.st_dev:
206+
log.debug(
207+
"Destination URI %s is the same file as source URI %s, returning immediately."
208+
" No further action required.",
209+
self,
210+
src,
211+
)
212+
return
213+
214+
if not overwrite and dest_stat:
215+
raise FileExistsError(
216+
f"Destination path '{self}' already exists. Transfer from {src} cannot be completed."
217+
)
218+
219+
# Make the destination path absolute (but don't follow links since
220+
# that would possibly cause us to end up in the wrong place if the
221+
# file existed already as a soft link)
222+
newFullPath = os.path.abspath(self.ospath)
223+
outputDir = os.path.dirname(newFullPath)
224+
185225
# We do not have to special case FileResourcePath here because
186226
# as_local handles that. If remote download, download it to the
187-
# destination directory to allow an atomic rename.
188-
with src.as_local(multithreaded=multithreaded, tmpdir=self.dirname()) as local_uri:
227+
# destination directory to allow an atomic rename but only if that
228+
# directory exists because we do not want to create a directory
229+
# but then end up with the download failing.
230+
tmpdir = outputDir if os.path.exists(outputDir) else None
231+
with src.as_local(multithreaded=multithreaded, tmpdir=tmpdir) as local_uri:
189232
is_temporary = local_uri.isTemporary
190233
local_src = local_uri.ospath
191234

@@ -239,45 +282,6 @@ def transfer_from(
239282
if src != local_uri and is_temporary and transfer == "copy":
240283
transfer = "move"
241284

242-
# The output location should not exist unless overwrite=True.
243-
# Rather than use `exists()`, use os.stat since we might need
244-
# the full answer later.
245-
dest_stat: os.stat_result | None
246-
try:
247-
# Do not read through links of the file itself.
248-
dest_stat = os.lstat(self.ospath)
249-
except FileNotFoundError:
250-
dest_stat = None
251-
252-
# It is possible that the source URI and target URI refer
253-
# to the same file. This can happen for a number of reasons
254-
# (such as soft links in the path, or they really are the same).
255-
# In that case log a message and return as if the transfer
256-
# completed (it technically did). A temporary file download
257-
# can't be the same so the test can be skipped.
258-
if dest_stat and not is_temporary:
259-
# Be consistent and use lstat here (even though realpath
260-
# has been called). It does not harm.
261-
local_src_stat = os.lstat(local_src)
262-
if dest_stat.st_ino == local_src_stat.st_ino and dest_stat.st_dev == local_src_stat.st_dev:
263-
log.debug(
264-
"Destination URI %s is the same file as source URI %s, returning immediately."
265-
" No further action required.",
266-
self,
267-
local_uri,
268-
)
269-
return
270-
271-
if not overwrite and dest_stat:
272-
raise FileExistsError(
273-
f"Destination path '{self}' already exists. Transfer from {src} cannot be completed."
274-
)
275-
276-
# Make the path absolute (but don't follow links since that
277-
# would possibly cause us to end up in the wrong place if the
278-
# file existed already as a soft link)
279-
newFullPath = os.path.abspath(self.ospath)
280-
outputDir = os.path.dirname(newFullPath)
281285
if not os.path.isdir(outputDir):
282286
# Must create the directory -- this can not be rolled back
283287
# since another transfer running concurrently may

0 commit comments

Comments
 (0)