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.

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

[BUG] [flytekit] _ModuleSanitizer returns a wrong name when site-packages/__init__.py exists. #6180

Open
2 tasks done
amitani opened this issue Jan 18, 2025 · 0 comments
Open
2 tasks done
Labels
bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers

Comments

@amitani
Copy link
Contributor

amitani commented Jan 18, 2025

Describe the bug

When a library puts __init__.py directly under the site-packages directory (maybe accidentally), _ModuleSanitizer.get_absolute_module_name doesn't return the correct absolute module name. For example, for flytekit.core.python_auto_container.default_task_resolver, it returns site-packages.flytekit.core.python_auto_container.default_task_resolver and this causes pyflyte-execute to fail because it's passed as --resolver argument.

I encountered this when torchsurv was installed.
Not sure if this is an issue Flyte needs to solve, or this library needs to be updated. Still, I'm currently deleting site-packages/__init__.py after installing the library, and it is brittle.

A few fixes I can think of is,

  1. In addition to checking if __init__.py doesn't exist in the dir, it can end the recursion when the dirname matches what's in sys.path.
  2. Handle flytekit as a special case in the sanitizer.

I'm not very familiar with this part, so my suggestions may break other usage, though. Happy to contribute a PR. Some pointers are really appreciated.

(a separate issue, but a counter example in the other direction may be namespace packages. a dir without __init__.py doesn't mean it's the root)

Expected behavior

_ModuleSanitizer.get_absolute_module_name always returns flytekit.core.python_auto_container.default_task_resolver for the default task resolver, even when site-packages/__init__.py exists.

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
@amitani amitani added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels Jan 18, 2025
@amitani amitani changed the title [BUG] [flytekit] _ModuleSanitizer fails when site-packages/__init__.py exists. [BUG] [flytekit] _ModuleSanitizer returns a wrong name when site-packages/__init__.py exists. Jan 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers
Projects
Status: Backlog
Development

No branches or pull requests

1 participant