Skip to content

Rename pip_parse to pip_install #757

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
wants to merge 13 commits into from
Closed

Rename pip_parse to pip_install #757

wants to merge 13 commits into from

Conversation

groodt
Copy link
Collaborator

@groodt groodt commented Jul 14, 2022

This is a fairly large refactor and unfortunately some breaking changes. The intent here is to reduce complexity and duplication.

Historically, we had 2 rules to install packages from PyPI (pip_install and pip_parse), with pip_install being the original. It worked well, but had a significant flaw in that it would eagerly fetch and install all packages. This was slow and frustrating when there were a large number of dependencies, so pip_parse was born!

pip_parse (even though it is still a repository rule) lazily fetched and installed packages. This was adopted by most of the community based on evidence from issues raised, the bazel Slack channels and the rule maintainers.

Maintaining both versions should no longer be necessary. There was a lot of duplication in both versions and it required extra effort and consideration for adding any new features.

I've taken the approach (probably 🌶️ ) that pip_install is a better name, so I've introduced this change where both pip_install and pip_parse share the same code, but with a small shim to handle the requirements_lock vs requirements difference in argument. I think a deprecation log message should be added and then pip_parse should be removed.

2 Breaking Changes:

  1. The small breaking change in this PR, will be for the users (probably a small number) who still use a pip_install version prior to this PR and who DO NOT use the requirement() macro to reference dependencies. The naming style of the external repositories has changed, so this will require a sed across any .bazel files if these users upgrade. This isn't a big deal in my opinion.

@groodt groodt mentioned this pull request Aug 27, 2022
@groodt
Copy link
Collaborator Author

groodt commented Aug 28, 2022

Closing. Proceeding with smaller and less extreme approach here. #807

@groodt groodt closed this Aug 28, 2022
@aignas aignas deleted the groodt-rename-pip-parse branch October 17, 2024 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants