Skip to content

Commit

Permalink
fix: Prevent local files from being moved when using FlyteFile on loc…
Browse files Browse the repository at this point in the history
…al executions (flyteorg#2476)

* fix: Do not copy local files when using FlyteFile

Signed-off-by: ggydush <[email protected]>

* fix: Prevent copying of local files when running local execution

Signed-off-by: ggydush <[email protected]>

* fix: Revert

Signed-off-by: ggydush <[email protected]>

* fix: Fix another location of should upload

Signed-off-by: ggydush <[email protected]>

* test: Fix failing test cases

Signed-off-by: ggydush <[email protected]>

* fix: Fix to still handle uploads

Signed-off-by: ggydush <[email protected]>

---------

Signed-off-by: ggydush <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Signed-off-by: bugra.gedik <[email protected]>
  • Loading branch information
2 people authored and bugra.gedik committed Jul 3, 2024
1 parent 803a128 commit b29b944
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 7 deletions.
5 changes: 5 additions & 0 deletions flytekit/types/file/file.py
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,9 @@ def to_literal(
# Set the remote destination if one was given instead of triggering a random one below
remote_path = python_val.remote_path or None

if ctx.execution_state.is_local_execution and python_val.remote_path is None:
should_upload = False

elif isinstance(python_val, pathlib.Path) or isinstance(python_val, str):
source_path = str(python_val)
if issubclass(python_type, FlyteFile):
Expand All @@ -455,6 +458,8 @@ def to_literal(
p = pathlib.Path(python_val)
if not p.is_file():
raise TypeTransformerFailedError(f"Error converting {python_val} because it's not a file.")
if ctx.execution_state.is_local_execution:
should_upload = False
# python_type must be os.PathLike - see check at beginning of function
else:
should_upload = False
Expand Down
9 changes: 2 additions & 7 deletions tests/flytekit/unit/core/test_flyte_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ def my_wf(fname: os.PathLike = SAMPLE_DATA) -> int:
assert sample_lp.parameters.parameters["fname"].default.scalar.blob.uri == SAMPLE_DATA


def test_file_handling_local_file_gets_copied():
def test_file_handling_local_file_does_not_get_copied():
@task
def t1() -> FlyteFile:
# Use this test file itself, since we know it exists.
Expand All @@ -223,12 +223,7 @@ def my_wf() -> FlyteFile:
assert len(top_level_files) == 1 # the flytekit_local folder

x = my_wf()

# After running, this test file should've been copied to the mock remote location.
mock_remote_files = os.listdir(os.path.join(random_dir, "mock_remote"))
assert len(mock_remote_files) == 1 # the file
# File should've been copied to the mock remote folder
assert x.path.startswith(random_dir)
assert x.path == __file__


def test_file_handling_local_file_gets_force_no_copy():
Expand Down

0 comments on commit b29b944

Please sign in to comment.