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

refactor(python): support multiple requirements.in #56

Merged
merged 6 commits into from
Jun 23, 2024
Merged

Conversation

alexeagle
Copy link
Member

@alexeagle alexeagle commented Jun 16, 2024

This allows the non-Bazel workflow to still pin or install test dependencies in isolation.

Note that install of pytest is intentional to prepare for adding some py_test workflow next.

Now that there's yet another update command required when dependencies change, I've introduced a repin.sh script. This can be the cheap version of aspect-build/aspect-cli#687 - when we like the DX I can bake the result into Aspect CLI.

@alexeagle alexeagle force-pushed the pyproject_deps branch 3 times, most recently from 29ac674 to cdf7d39 Compare June 16, 2024 15:51
@alexeagle
Copy link
Member Author

@jab FYI, does this meet your expectations? Note that I don't bother pinning test.txt here since Bazel wouldn't need to read it - but if you ran some external tooling you might still want that created?

Also LMK if you're interested in being a collaborator on this repo!

@alexeagle alexeagle force-pushed the pyproject_deps branch 4 times, most recently from 07078ee to dd48064 Compare June 16, 2024 16:04
This allows the non-Bazel workflow to still pin or install test dependencies in isolation
@alexeagle alexeagle requested a review from mattem June 16, 2024 16:10
@alexeagle alexeagle marked this pull request as ready for review June 16, 2024 16:10
Copy link
Contributor

@jab jab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Only blocker is a surviving reference to //requirements:requirements.update which now needs to change to //requirements:runtime.update.

{{ .ProjectSnake }}/repin.sh Outdated Show resolved Hide resolved
{{ .ProjectSnake }}/BUILD.bazel Show resolved Hide resolved
{{ .ProjectSnake }}/README.bazel.md Show resolved Hide resolved
{{ .ProjectSnake }}/requirements/all.in Outdated Show resolved Hide resolved
@jab
Copy link
Contributor

jab commented Jun 17, 2024

Note that I don't bother pinning test.txt here since Bazel wouldn't need to read it - but if you ran some external tooling you might still want that created?

My hunch is that starting with just runtime.txt and all.txt (as you've done here) sufficiently covers the main goal here, and it's nice to avoid the complexity of an additional test.txt. Not unreasonable to expect a user who needs that intermediate lockfile to figure out how to generate it based on what's now generated out-of-the-box, IMO.

Also LMK if you're interested in being a collaborator on this repo!

Sure, thanks!

alexeagle and others added 5 commits June 23, 2024 13:14
Co-authored-by: Joshua Bronson <jabronson@gmail.com>
Co-authored-by: Joshua Bronson <jabronson@gmail.com>
Co-authored-by: Joshua Bronson <jabronson@gmail.com>
@alexeagle alexeagle merged commit f516e0c into main Jun 23, 2024
6 checks passed
@alexeagle alexeagle deleted the pyproject_deps branch June 23, 2024 20:22
requirements_in = "pyproject.toml",
requirements_txt = "requirements_lock.txt",
)

# Fetches metadata for python packages we depend on.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexeagle, were these changes missing

+exports_files(["pyproject.toml"])

?

I just tried aspect init for the first time since this PR was merged. Immediately running ./repin.sh (making no changes to what was generated) resulted in

ERROR: /private/var/folders/_t/xpfjyxwx727_zrtwcrhyfyjm0000gn/T/tmp.uBVmFZpqkL/foo/BUILD.bazel: no such target '//:pyproject.toml': target 'pyproject.toml' not declared in package '' defined by /private/var/folders/_t/xpfjyxwx727_zrtwcrhyfyjm0000gn/T/tmp.uBVmFZpqkL/foo/BUILD.bazel; however, a source file of this name exists.  (Perhaps add 'exports_files(["pyproject.toml"])' to /BUILD?)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, some missing tests then. PR should have been red. I'll try to add this week

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.

2 participants