Skip to content

Namespace packages do not always work as expected #1242

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

Closed
aignas opened this issue May 29, 2023 · 3 comments
Closed

Namespace packages do not always work as expected #1242

aignas opened this issue May 29, 2023 · 3 comments
Labels
Can Close? Will close in 30 days if there is no new activity

Comments

@aignas
Copy link
Collaborator

aignas commented May 29, 2023

🐞 bug report

Affected Rule

py_library, py_test, py_binary

Is this a regression?

It is likely that it was working like this all the time, so it's not a regression. What is more, I am wondering if this is actually a bug in how Airflow is packaged as the more I am describing this issue, the more it seems to be the case.

Description

I have reproduced the behaviour that I was seeing when using airflow within bazel. It seems that the airflow package is relying on namespace pkgs feature by splitting the whole airflow package into smaller ones and making providers non-mandatory to be installed on the system, e.g. see the sqlite provider: https://pypi.org/project/apache-airflow-providers-sqlite/. airflow also relies on lazy-loading some of the core classes from the main package and thus we may have a problem - the lazy loading does not work if the sys.path entry for the provider happens to be before the airflow entry. This is because it seems that Python is going to use the __getattr__ function declared in the main module only if the main module is the first one in the path.

My assumption is that the majority of the installs out there are not hitting this edge-case because everybody is working in a single virtualenv which contains all of the airflow providers and airflow itself and I am wondering if there is something we should do to workaround the issue. At the very least this will document it and a possible workaround for it. For now what we are doing is we are sorting the sys.path elements alphabetically before importing anything from airflow and this achieves the effect that we want.

@rickeylev, do you know how the order of sys.path elements within the bazel/rules_python is determined? Given the fact that most of the times buildifier is sorting the dependencies and this may affect the order of the sys.path elements if I understand things correctly? Do you think having a virtualenv layout of the runfiles would be something that rules_python could support?

🔬 Minimal Reproduction

A reproduction of the behaviour is documented in https://github.com/aignas/rules_python/tree/test/namespace_pkgs/examples/namespace_pkgs

@aignas
Copy link
Collaborator Author

aignas commented May 30, 2023

Thinking about it more, I think it is a bug/limitation in Airflow, but I am we may notice more cases in the future and I am wondering if having some predefined order in the sys.path entries within the entrypoint template would be a good thing here.

Copy link

This issue has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days.
Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_python!

@github-actions github-actions bot added the Can Close? Will close in 30 days if there is no new activity label Nov 26, 2023
Copy link

This issue was automatically closed because it went 30 days without a reply since it was labeled "Can Close?"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Can Close? Will close in 30 days if there is no new activity
Projects
None yet
Development

No branches or pull requests

1 participant