Skip to content
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.

Sign up for GitHub

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

fix: Prevent local files from being moved when using FlyteFile on local executions #2476

Merged
merged 7 commits into from
Jun 27, 2024
Merged

fix: Prevent local files from being moved when using FlyteFile on local executions #2476

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
92b2c4a
fix: Do not copy local files when using FlyteFile
ggydush Jun 12, 2024
35a0b60
fix: Prevent copying of local files when running local execution
ggydush Jun 12, 2024
f60bda7
fix: Revert
ggydush Jun 12, 2024
9097e69
fix: Fix another location of should upload
ggydush Jun 12, 2024
9e2510c
test: Fix failing test cases
ggydush Jun 12, 2024
47bd9e7
fix: Fix to still handle uploads
ggydush Jun 13, 2024
078a47d
Merge remote-tracking branch 'origin' into ggydush/fix-flytefile-loca…
eapolinario Jun 26, 2024
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
5 changes: 5 additions & 0 deletions flytekit/types/file/file.py
View file
Open in desktop
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
View file
Open in desktop
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
Loading