search

bazelbuild / rules_python

Bazel Python Rules
https://rules-python.readthedocs.io
Apache License 2.0
511 stars 518 forks source link

Promoting `experimental_requirement_cycles` from experimental #1663

Open Philip-Murzynowski opened 6 months ago

Philip-Murzynowski commented 6 months ago

🚀 feature request

Relevant Rules

I have been making use of the experimental_requirement_cycles parameter for the pip_parse rule and greatly benefiting from it. I wanted to ask if there is a release plan or anything I could look into to help incorporate it into a release.

Description

The experimental feature is introduced [here](https://github.com/bazelbuild/rules_python/blob/4c2d7d9d6608795522322a9b2294b5053e41b979/CHANGELOG.md?plain=1#L31) in the CHANGELOG. I saw that recommendation for requesting a feature to be taken out of experimental was to open an issue as noted in the [support.md](https://github.com/bazelbuild/rules_python/blob/df234d9a696cb1d23a5612e759602465fa6aef71/docs/sphinx/support.md#experimental-features). ### Describe the solution you'd like As this is my first activity in this repository, please let me know if you would like more information. ### Describe alternatives you've considered
arrdem commented 5 months ago

As the primary author of that feature I'm 50/50 on promoting it to stable. It's a fine solution for making true dependency cycles work (eg. a <-> b in all cases of extras requirements). I'm less convinced that it's a good solution for making requirements using extras work since we have to convert what should be optional dependencies into strong dependencies.

Consider a[] requiring b with b requiring a, and a[c] requiring c with c requiring a. py_requirement("a") is now ambiguous. Does the user intend a[c]? a[]? Do we intend the requirements.txt locks to function as virtualenvs from which everything* is installed or as a dependency solution set from which only the needed requirements should be installed?

Personally I prefer the latter. I think having to merge all the Airflow optional dependencies into a single requirement group isn't a great solution because it means that any artifact which requires Airflow takes a dependency on the entire group of requirements. It would be better if say the mysql extra wasn't always implied because py_requirement("airflow") now means the entire group with all extras.

There's plenty of room to critique setuptools/pip here for extras really behaving as just sneaky transitive deps not proper package configuration https://discuss.python.org/t/provide-information-on-installed-extras-in-package-metadata/15385.

There's the related problem of needing to merge packaging cycles when there are multiple, again see the newer Airflow packaging. Is that really the right behavior? Or could we do something clever to break these cycles into separate components which would behave more reasonably as optional extras?

aignas commented 5 days ago

Some thoughts about a better solution here: https://github.com/bazelbuild/rules_python/pull/1728#discussion_r1660617136

Not sure yet how to integrate the current experimental_index_url with the code snippet in there, but it would be an alternative that would not require the users to specify the experimental_requirement_cycles at all and it would be my preferred solution.