[BUG] FlyteDirectory.listdir does not return full paths for local files #6005

pimdh · 2024-11-13T14:29:48Z

Describe the bug

Thanks for this great library!

For local files, FlyteDirectory.listdir returns only the names of the files/dirs in the given directory, not the full paths. This breaks the following workflow.

import tempfile
from pathlib import Path

from flytekit import FlyteDirectory, FlyteFile, map_task, task, workflow


@task
def setup() -> FlyteDirectory:
    tmpdir = Path(tempfile.mkdtemp())
    for i in range(3):
        (tmpdir / f"file{i}.txt").write_text(f"Hello, World! {i}")
    return FlyteDirectory(tmpdir)


@task
def read_file(file: FlyteFile) -> str:
    with open(file, "r") as f:
        return f.read()


@task
def list_dir(dir: FlyteDirectory) -> list[FlyteFile]:
    return FlyteDirectory.listdir(dir)


@workflow
def wf() -> list[str]:
    tmpdir = setup()
    files = list_dir(dir=tmpdir)
    return map_task(read_file)(file=files)

Run with pyflyte run debug_flyte.py wf.

This issue arises because only the file/dir name is returned, not the joined path here.

Expected behavior

I would expect to get the outputs:

['Hello, World! 0', 'Hello, World! 2', 'Hello, World! 1']

(in some order), but instead I get

Running Execution on local.
FlyteAssertion: USER:AssertionError: error=Error encountered while converting inputs of 'debug_flyte.map_read_file_6b3bd0353da5de6e84d7982921ead2b3-arraynode':
  Cannot convert from Flyte Serialized object (Literal):
  scalar:
    blob:
      metadata:
      uri: file0.txt to <class 'flytekit.types.file.file.FlyteFile'>. Expected a file, but file0.txt is not a file.

Additional context to reproduce

No response

Screenshots

No response

Are you sure this issue hasn't been raised already?

Yes

Have you read the Code of Conduct?

Yes

The text was updated successfully, but these errors were encountered:

welcome · 2024-11-13T14:29:51Z

Thank you for opening your first issue here! 🛠

Fixes flyteorg/flyte#6005

Fixes flyteorg/flyte#6005 Signed-off-by: Pim de Haan <[email protected]>

* Store protos in local cache (#3022) * Store proto obj instead of model Literal in local cache Signed-off-by: Eduardo Apolinario <[email protected]> * Remove unused file Signed-off-by: Eduardo Apolinario <[email protected]> --------- Signed-off-by: Eduardo Apolinario <[email protected]> Co-authored-by: Eduardo Apolinario <[email protected]> * Bump aiohttp from 3.9.5 to 3.10.11 (#3018) Bumps [aiohttp](https://github.com/aio-libs/aiohttp) from 3.9.5 to 3.10.11. - [Release notes](https://github.com/aio-libs/aiohttp/releases) - [Changelog](https://github.com/aio-libs/aiohttp/blob/master/CHANGES.rst) - [Commits](aio-libs/aiohttp@v3.9.5...v3.10.11) --- updated-dependencies: - dependency-name: aiohttp dependency-type: indirect ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix bug in FlyteDirectory.listdir on local files (#2926) * Fix issue in FlyteDirectory.listdir Fixes flyteorg/flyte#6005 Signed-off-by: Pim de Haan <[email protected]> * Added test Signed-off-by: Pim de Haan <[email protected]> * Run make lint Signed-off-by: Eduardo Apolinario <[email protected]> --------- Signed-off-by: Pim de Haan <[email protected]> Signed-off-by: Eduardo Apolinario <[email protected]> Co-authored-by: Eduardo Apolinario <[email protected]> * Fix unit tests in airflow plugin (#3024) Signed-off-by: Kevin Su <[email protected]> * fix: Fix resource meta typos for async agent (#3023) Signed-off-by: JiaWei Jiang <[email protected]> * fix: format commands output (#3026) * Fix pydantic basemodel default input (#3013) * Fix pydantic default input Signed-off-by: Future-Outlier <[email protected]> * add pydantic integration test Signed-off-by: Future-Outlier <[email protected]> * Use duck typing by Thomas's advice Signed-off-by: Future-Outlier <[email protected]> Co-authored-by: Thomas J. Fan <[email protected]> * lint Signed-off-by: Future-Outlier <[email protected]> --------- Signed-off-by: Future-Outlier <[email protected]> Co-authored-by: Thomas J. Fan <[email protected]> * [BUG] Open FlyteFile from remote path (#2991) * fix: Open FlyteFile from remote path Signed-off-by: JiaWei Jiang <[email protected]> * Add integration test Signed-off-by: JiaWei Jiang <[email protected]> * refactor: Use ctx as param instead of recreation Signed-off-by: JiaWei Jiang <[email protected]> * refactor: Clean test logic 1. Remove redundant prints 2. Use `mock.patch.dict` to setup `os.environ` for the current test fn * Avoid contaminating other tests running in the same process Signed-off-by: JiaWei Jiang <[email protected]> * refactor: Setup local path and downloader in constructor Signed-off-by: JiaWei Jiang <[email protected]> * refactor: Move SimpleFileTransfer to an utility file Signed-off-by: JiaWei Jiang <[email protected]> * Remove redundant env var setup Please refer to #3001 Signed-off-by: JiaWei Jiang <[email protected]> * test: Add another ff use case Create ff in one task pod and read it in another task pod. Signed-off-by: JiaWei Jiang <[email protected]> --------- Signed-off-by: JiaWei Jiang <[email protected]> * vllm inference plugin (#2967) * vllm inference plugin Signed-off-by: Daniel Sola <[email protected]> * fixed default value Signed-off-by: Daniel Sola <[email protected]> --------- Signed-off-by: Daniel Sola <[email protected]> * Add poetry to image spec (#3025) * Add poetry to image spec Signed-off-by: Thomas J. Fan <[email protected]> * Add stricter check Signed-off-by: Thomas J. Fan <[email protected]> --------- Signed-off-by: Thomas J. Fan <[email protected]> * [test] Add integration test for accessing sd sttr in dc (#2969) * test: Add integration test for attr access of sd Signed-off-by: JiaWei Jiang <[email protected]> * Correct file path Signed-off-by: JiaWei Jiang <[email protected]> * test: Support interaction with minio s3 bucket 1. Upload a local parquet file to minio s3 bucket 2. Access StructuredDataset attr from a dataclass 3. Open StructuredDataset from a remote path Signed-off-by: JiaWei Jiang <[email protected]> * Delete an unmerged integration test Signed-off-by: JiaWei Jiang <[email protected]> * Try imagespec with commit sha of corresponding fix Signed-off-by: JiaWei Jiang <[email protected]> * Remove redundant test Signed-off-by: JiaWei Jiang <[email protected]> * Remove default_factory and create sd dc from input uri Signed-off-by: JiaWei Jiang <[email protected]> * refactor: Clean test logic 1. Remove redundant prints 2. Use `mock.patch.dict` to setup `os.environ` for the current test fn * Avoid contaminating other tests running in the same process Signed-off-by: JiaWei Jiang <[email protected]> * Remove redundant minio env var setup and add test comments Signed-off-by: JiaWei Jiang <[email protected]> * Support uploading tmp pqt file Signed-off-by: JiaWei Jiang <[email protected]> * Udpate deprecated module Signed-off-by: JiaWei Jiang <[email protected]> * Remove redundant and unused imports Signed-off-by: JiaWei Jiang <[email protected]> --------- Signed-off-by: JiaWei Jiang <[email protected]> --------- Signed-off-by: Eduardo Apolinario <[email protected]> Signed-off-by: dependabot[bot] <[email protected]> Signed-off-by: Pim de Haan <[email protected]> Signed-off-by: Kevin Su <[email protected]> Signed-off-by: JiaWei Jiang <[email protected]> Signed-off-by: Future-Outlier <[email protected]> Signed-off-by: Daniel Sola <[email protected]> Signed-off-by: Thomas J. Fan <[email protected]> Co-authored-by: Eduardo Apolinario <[email protected]> Co-authored-by: Eduardo Apolinario <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Pim de Haan <[email protected]> Co-authored-by: Kevin Su <[email protected]> Co-authored-by: 江家瑋 <[email protected]> Co-authored-by: V <[email protected]> Co-authored-by: Han-Ru Chen (Future-Outlier) <[email protected]> Co-authored-by: Thomas J. Fan <[email protected]> Co-authored-by: Daniel Sola <[email protected]>

* Fix issue in FlyteDirectory.listdir Fixes flyteorg/flyte#6005 Signed-off-by: Pim de Haan <[email protected]> * Added test Signed-off-by: Pim de Haan <[email protected]> * Run make lint Signed-off-by: Eduardo Apolinario <[email protected]> --------- Signed-off-by: Pim de Haan <[email protected]> Signed-off-by: Eduardo Apolinario <[email protected]> Co-authored-by: Eduardo Apolinario <[email protected]> Signed-off-by: Shuying Liang <[email protected]>

pimdh added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels Nov 13, 2024

github-project-automation bot added this to Flyte Issues/PRs maintenance Nov 13, 2024

github-project-automation bot moved this to Backlog in Flyte Issues/PRs maintenance Nov 13, 2024

pimdh added a commit to pimdh/flytekit that referenced this issue Nov 13, 2024

Fix issue in FlyteDirectory.listdir

48804b8

Fixes flyteorg/flyte#6005

pimdh mentioned this issue Nov 13, 2024

Fix bug in FlyteDirectory.listdir on local files flyteorg/flytekit#2926

Merged

3 tasks

pimdh added a commit to pimdh/flytekit that referenced this issue Nov 13, 2024

Fix issue in FlyteDirectory.listdir

c063eae

Fixes flyteorg/flyte#6005 Signed-off-by: Pim de Haan <[email protected]>

eapolinario removed the untriaged This issues has not yet been looked at by the Maintainers label Nov 14, 2024

eapolinario self-assigned this Nov 14, 2024

eapolinario closed this as completed in flyteorg/flytekit#2926 Dec 27, 2024

eapolinario closed this as completed in flyteorg/flytekit@edbf992 Dec 27, 2024

github-project-automation bot moved this from Backlog to Done in Flyte Issues/PRs maintenance Dec 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUG] FlyteDirectory.listdir does not return full paths for local files #6005

[BUG] FlyteDirectory.listdir does not return full paths for local files #6005

pimdh commented Nov 13, 2024

welcome bot commented Nov 13, 2024

[BUG] FlyteDirectory.listdir does not return full paths for local files #6005

[BUG] FlyteDirectory.listdir does not return full paths for local files #6005

Comments

pimdh commented Nov 13, 2024

Describe the bug

Expected behavior

Additional context to reproduce

Screenshots

Are you sure this issue hasn't been raised already?

Have you read the Code of Conduct?

welcome bot commented Nov 13, 2024