8000 Simplify implementation of temporary_path() by Tarrasch · Pull Request #2652 · spotify/luigi · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Simplify implementation of temporary_path() #2652

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 16 additions & 25 deletions luigi/target.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import tempfile
import logging
import warnings
from contextlib import contextmanager
from luigi import six

logger = logging.getLogger('luigi-interface')
Expand Down Expand Up @@ -250,6 +251,7 @@ def remove(self):
"""
self.fs.remove(self.path)

@contextmanager
def temporary_path(self):
"""
A context manager that enables a reasonably short, general and
Expand All @@ -275,31 +277,20 @@ def run(self):
with self.output().temporary_path() as self.temp_output_path:
run_some_external_command(output_path=self.temp_output_path)
"""
class _Manager(object):
target = self

def __init__(self):
num = random.randrange(0, 1e10)
slashless_path = self.target.path.rstrip('/').rstrip("\\")
self._temp_path = '{}-luigi-tmp-{:010}{}'.format(
slashless_path,
num,
self.target._trailing_slash())
# TODO: os.path doesn't make sense here as it's os-dependent
tmp_dir = os.path.dirname(slashless_path)
if tmp_dir:
self.target.fs.mkdir(tmp_dir, parents=True, raise_if_exists=False)

def __enter__(self):
return self._temp_path

def __exit__(self, exc_type, exc_value, traceback):
if exc_type is None:
# There were no exceptions
self.target.fs.rename_dont_move(self._temp_path, self.target.path)
return False # False means we don't suppress the exception

return _Manager()
num = random.randrange(0, 1e10)
slashless_path = self.path.rstrip('/').rstrip("\\")
_temp_path = '{}-luigi-tmp-{:010}{}'.format(
slashless_path,
num,
self._trailing_slash())
# TODO: os.path doesn't make sense here as it's os-dependent
tmp_dir = os.path.dirname(slashless_path)
if tmp_dir:
self.fs.mkdir(tmp_dir, parents=True, raise_if_exists=False)

yield _temp_path
# We won't reach here if there was an user exception.
self.fs.rename_dont_move(_temp_path, self.path)

def _touchz(self):
with self.open('w'):
Expand Down
15 changes: 8 additions & 7 deletions test/target_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -294,20 +294,21 @@ class MyException(Exception):
orig_ex = MyException()
try:
with target_outer.temporary_path() as tmp_path_outer:
assert 'notreal' in tmp_path_outer
self.assertIn('notreal', tmp_path_outer)
with target_inner.temporary_path() as tmp_path_inner:
assert 'blah' in tmp_path_inner
self.assertIn('blah', tmp_path_inner)
with target_inner.temporary_path() as tmp_path_inner_2:
assert tmp_path_inner != tmp_path_inner_2
self.fs.rename_dont_move.assert_called_once_with(tmp_path_inner_2, target_inner.path)
self.assertNotEqual(tmp_path_inner, tmp_path_inner_2)
self.fs.rename_dont_move.assert_called_once_with(tmp_path_inner_2,
target_inner.path)
self.fs.rename_dont_move.assert_called_with(tmp_path_inner, target_inner.path)
self.fs.rename_dont_move.call_count == 2
self.assertEqual(self.fs.rename_dont_move.call_count, 2)
raise orig_ex
except MyException as ex:
self.fs.rename_dont_move.call_count == 2
assert ex is orig_ex
self.assertIs(ex, orig_ex)
else:
assert False
self.assertEqual(self.fs.rename_dont_move.call_count, 2)

def test_temporary_path_directory(self):
target_slash = self.target_cls('/tmp/dir/')
Expand Down
0