diff --git a/luigi/contrib/dropbox.py b/luigi/contrib/dropbox.py index 76760b8688..3a4a3310f7 100644 --- a/luigi/contrib/dropbox.py +++ b/luigi/contrib/dropbox.py @@ -68,9 +68,10 @@ class DropboxClient(FileSystem): Dropbox client for authentication, designed to be used by the :py:class:`DropboxTarget` class. """ - def __init__(self, token, user_agent="Luigi"): + def __init__(self, token, user_agent="Luigi", root_namespace_id=None): """ :param str token: Dropbox Oauth2 Token. See :class:`DropboxTarget` for more information about generating a token + :param str root_namespace_id: Root namespace ID for interacting with Team Spaces """ if not token: raise ValueError("The token parameter must contain a valid Dropbox Oauth2 Token") @@ -80,6 +81,9 @@ def __init__(self, token, user_agent="Luigi"): except Exception as e: raise Exception("Cannot connect to Dropbox. Check your Internet connection and the token. \n" + repr(e)) + if root_namespace_id: + conn = conn.with_path_root(dropbox.common.PathRoot.root(root_namespace_id)) + self.token = token self.conn = conn @@ -257,7 +261,7 @@ class DropboxTarget(FileSystemTarget): A Dropbox filesystem target. """ - def __init__(self, path, token, format=None, user_agent="Luigi"): + def __init__(self, path, token, format=None, user_agent="Luigi", root_namespace_id=None): """ Create an Dropbox Target for storing data in a dropbox.com account @@ -285,6 +289,7 @@ def __init__(self, path, token, format=None, user_agent="Luigi"): :param str path: Remote path in Dropbox (starting with '/'). :param str token: a valid OAuth2 Dropbox token. :param luigi.Format format: the luigi format to use (e.g. `luigi.format.Nop`) + :param str root_namespace_id: Root namespace ID for interacting with Team Spaces """ @@ -295,7 +300,7 @@ def __init__(self, path, token, format=None, user_agent="Luigi"): self.path = path self.token = token - self.client = DropboxClient(token, user_agent) + self.client = DropboxClient(token, user_agent, root_namespace_id) self.format = format or luigi.format.get_default_format() def __str__(self): diff --git a/luigi/contrib/ftp.py b/luigi/contrib/ftp.py index d155e8ef8b..78a423704e 100644 --- a/luigi/contrib/ftp.py +++ b/luigi/contrib/ftp.py @@ -309,7 +309,7 @@ def get(self, path, local_path): self._close() - os.rename(tmp_local_path, local_path) + os.replace(tmp_local_path, local_path) def _sftp_get(self, path, tmp_local_path): self.conn.get(path, tmp_local_path) diff --git a/luigi/contrib/ssh.py b/luigi/contrib/ssh.py index beda1c9e23..9e7ee03821 100644 --- a/luigi/contrib/ssh.py +++ b/luigi/contrib/ssh.py @@ -270,7 +270,7 @@ def get(self, path, local_path): tmp_local_path = local_path + '-luigi-tmp-%09d' % random.randrange(0, 10_000_000_000) self._scp("%s:%s" % (self.remote_context._host_ref(), path), tmp_local_path) - os.rename(tmp_local_path, local_path) + os.replace(tmp_local_path, local_path) class AtomicRemoteFileWriter(luigi.format.OutputPipeProcessWrapper): diff --git a/luigi/local_target.py b/luigi/local_target.py index 5cdade2ec7..ec6ae78023 100644 --- a/luigi/local_target.py +++ b/luigi/local_target.py @@ -37,7 +37,7 @@ class atomic_file(AtomicLocalFile): """ def move_to_final_destination(self): - os.rename(self.tmp_path, self.path) + os.replace(self.tmp_path, self.path) def generate_tmp_path(self, path): return path + '-luigi-tmp-%09d' % random.randrange(0, 10_000_000_000) @@ -109,12 +109,12 @@ def move(self, old_path, new_path, raise_if_exists=False): if d and not os.path.exists(d): self.mkdir(d) try: - os.rename(old_path, new_path) + os.replace(old_path, new_path) except OSError as err: if err.errno == errno.EXDEV: new_path_tmp = '%s-%09d' % (new_path, random.randint(0, 999999999)) shutil.copy(old_path, new_path_tmp) - os.rename(new_path_tmp, new_path) + os.replace(new_path_tmp, new_path) os.remove(old_path) else: raise err diff --git a/luigi/parameter.py b/luigi/parameter.py index 41cc6bc32f..51f6f83802 100644 --- a/luigi/parameter.py +++ b/luigi/parameter.py @@ -1384,7 +1384,12 @@ def parse(self, x): # loop required to parse tuple of tuples return tuple(self._convert_iterable_to_tuple(x) for x in json.loads(x, object_pairs_hook=FrozenOrderedDict)) except (ValueError, TypeError): - return tuple(literal_eval(x)) # if this causes an error, let that error be raised. + result = literal_eval(x) + # t_str = '("abcd")' + # Ensure that the result is not a string to avoid cases like ('a','b','c','d') + if isinstance(result, str): + raise ValueError("Parsed result cannot be a string") + return tuple(result) # if this causes an error, let that error be raised. def _convert_iterable_to_tuple(self, x): if isinstance(x, str): diff --git a/test/local_target_test.py b/test/local_target_test.py index d9c78539c5..1193d67d6f 100644 --- a/test/local_target_test.py +++ b/test/local_target_test.py @@ -152,16 +152,16 @@ def rename_across_filesystems(src, dst): err.errno = EXDEV raise err - real_rename = os.rename + real_rename = os.replace - def mockrename(src, dst): + def mockreplace(src, dst): if '-across-fs' in src: real_rename(src, dst) else: rename_across_filesystems(src, dst) copy = '%s-across-fs' % self.copy - with mock.patch('os.rename', mockrename): + with mock.patch('os.replace', mockreplace): t.move(copy) self.assertFalse(os.path.exists(self.path)) diff --git a/test/parameter_test.py b/test/parameter_test.py index 625a1f200d..622aff5b17 100644 --- a/test/parameter_test.py +++ b/test/parameter_test.py @@ -567,6 +567,18 @@ class Foo(luigi.Task): self.assertEqual(hash(Foo(args=('foo', 'bar')).args), hash(p.normalize(p.parse('["foo", "bar"]')))) + def test_tuple_invalid_string(self): + param = luigi.TupleParameter() + self.assertRaises(ValueError, lambda: param.parse('("abcd")')) + + def test_tuple_invalid_string_in_tuple(self): + param = luigi.TupleParameter() + self.assertRaises(ValueError, lambda: param.parse('(("abcd"))')) + + def test_parse_invalid_format(self): + param = luigi.TupleParameter() + self.assertRaises(SyntaxError, lambda: param.parse('((1,2),(3,4')) + def test_task(self): class Bar(luigi.Task): pass @@ -1225,6 +1237,7 @@ class LocalParameters1304Test(LuigiTestCase): https://github.com/spotify/luigi/issues/1304#issuecomment-148402284 """ + def test_local_params(self): class MyTask(RunOnceTask):