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

Bundle dependency requirements with release #381

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

camposandro
Copy link
Collaborator

@camposandro camposandro commented Aug 20, 2024

This pull request updates the publish-to-pypi.yml workflow, which runs on each package release, to include a file with the list of installed requirements at the time of publishing. Also, according to the following #362 (comment), I added an extra verification step to the pipeline, running the unit tests prior to packaging. Closes #362.

  • My PR includes a link to the issue that I am addressing

Usage details

The requirements file helps recreate environments with consistent transitive dependencies, and make deployments on Dask Kubernetes clusters reproducible. When installing hipscat-import on a Docker image one should:

$ pip download hipscat-import --no-deps --no-binary :all: # download the sdist
$ tar -xzf hipscat-import* # unzip it
$ cd hipscat-import*/
$ pip install -r requirements.txt # install the locked dependencies

Code Quality

  • I have read the Contribution Guide and LINCC Frameworks Code of Conduct
  • My code follows the code style of this project
  • My code builds (or compiles) cleanly without any errors or warnings
  • My code contains relevant comments and necessary documentation

Copy link

codecov bot commented Aug 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.73%. Comparing base (d87041a) to head (755a3e9).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #381   +/-   ##
=======================================
  Coverage   99.73%   99.73%           
=======================================
  Files          26       26           
  Lines        1496     1496           
=======================================
  Hits         1492     1492           
  Misses          4        4           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@johnny-gossick johnny-gossick left a comment

Choose a reason for hiding this comment

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

This looks like it should work! The EXTRA_PIP_PACKAGES environment variable might not support a requirements.txt file but if not I will try adding the script you supplied after && to extraArgs here for now. If that doesn't work, we will just build docker images with the dependencies in the requirements.txt file installed on top of the official dask images.

Copy link

@drewoldag drewoldag left a comment

Choose a reason for hiding this comment

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

This looks pretty tidy, nice work. We should definitely move this up to the PPT as well.

@camposandro camposandro merged commit 9eb59f1 into main Aug 22, 2024
10 checks passed
@camposandro camposandro deleted the issue/362/bundle-dependencies-lock branch August 22, 2024 17: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.

Bundle a poetry.lock file with the release
3 participants