[BUG] Flytekit misuses unexpected fsspec lpath result

[cgrass]

Describe the bug

fsspec has unexpected behavior in some scenarios. calling get with a URL with query params for rpath (remote path) and a filename for lpath (local path) results in lpath being used as a path dir. e.g., if rpath == http://www.ascii-art.de/ascii/s/startrek.txt?test=test2 and lpath == /var/output.pb a file gets written at /var/output.pb/startrek.txt?test=test2.

this causes an issue in the flytekit code because remote._get_output_literal_map() creates tmp_name with the filename ("output.pb"), and then uses tmp_name as the location for the resulting file. in reality, that path is a dir that contains the actual file. trying to open tmp_name results in a IsADirectoryError.

a discussion in slack can add more context.
This is related to another bug I filed and could potentially be fixed with the same solution.

Expected behavior

remote file gets written to /var/output.pb

Additional context to reproduce

You can see the fsspec behavior using the unit test below

def test_get_output():
    kwargs = {}
    spec = fsspec.implementations.http.HTTPFileSystem(fsspec.filesystem("https", **kwargs))
    frompath = linkToUrlFileile ### must be url that includes query params, e.g., for presigned urls.
    topath = "/var/inputs10.txt"
    response = spec.get(frompath, topath, **kwargs)
    print(response)

e2e testing is much harder because it requires a specific flyte configuration (described in this PR). @wild-endeavor was able to reproduce using a local setup.

Screenshots

No response

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

• Yes

Have you read the Code of Conduct?

• Yes

[welcome]

Thank you for opening your first issue here! 🛠

[dosubot]

🤖

Hello @cgrass! 👋 I'm Dosu, a friendly bot here to assist you while we wait for a human maintainer. I'm designed to help troubleshoot bugs, answer your questions, and guide you through the process of becoming a contributor. Just remember, I'm here to help make things smoother for you!

Thank you for the detailed bug report. It seems like the issue is related to the way fsspec handles the lpath parameter when using the get method. As you've pointed out, it treats the lpath as a directory rather than a file path, which leads to the IsADirectoryError when Flytekit tries to open it.

The remote._get_output_literal_map() function in Flytekit is used to retrieve the output of a task execution. It creates a temporary file (tmp_name) to store the output, but due to the unexpected behavior of fsspec, it ends up being a directory.

As for the specific Flyte configuration that makes end-to-end testing harder, it's the storage type being set to 'minio'. This configuration means that Flyte is using MinIO for storage, which is a high performance, Kubernetes-native object storage suite. While MinIO is great for production environments, it can make end-to-end testing more difficult because it requires a running MinIO server, which may not be available or practical in a testing environment. You can find more details about this configuration in the storage.yaml file.

I would recommend creating an issue in the fsspec repository to address this unexpected behavior. In the meantime, a possible workaround could be to modify the remote._get_output_literal_map() function to handle this scenario, perhaps by checking if tmp_name is a directory and, if so, appending the filename to it.

Please let me know if you need further assistance.

Sources

• kustomize/overlays/test/flyte/config/common/storage.yaml

---

This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

[wild-endeavor]

actually @cgrass could you check again? i'm not seeing this behavior using plain fsspec

def test_get_output():
    from fsspec.implementations.http import HTTPFileSystem

    spec = HTTPFileSystem(fsspec.filesystem("https"))
    frompath = "http://www.ascii-art.de/ascii/s/startrek.txt"
    topath = "/Users/ytong/temp/2063.txt"
    response = spec.get(frompath, topath)
    print(response)

which version of fsspec do you have? i'm on 2024.2.0

[cgrass]

EDIT: actually, let me do a little more testing before confirming the old behavior.

EDIT2: @wild-endeavor - i left off an important piece to the repro step. the remote URL must include query params. if you update the frompath in your test to be http://www.ascii-art.de/ascii/s/startrek.txt?test=test2 you will see the behavior described in the bug report. I'll update the repro steps to make that clear.

[wild-endeavor]

@cgrass thanks for the clarification! do you think your existing ticket on fsspec/filesystem_spec#1505 covers this? since this is a purely fsspec issue (like no flytekit is involved in the repro) it's a little hard to work around for us until it's fixed in fsspec.

[cgrass]

@wild-endeavor I agree the root problem is with fsspec behavior but there are a few options for flyte to harden the code and potentially get the implementation to work even with the current fsspec version.

1. try the list option (i see you already created a PR to test if that works)
2. update remote._get_output_literal_map(). I put together a simple POC when i first discovered the bug that was a little smarter about how tmp_name was created and passed around. that worked for my specific use case but needs to be tested for more general use.
3. use get_file instead of get when a single file is being fetched.