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.

Sign up for GitHub

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

Jump to bottom

fix: Remove uv cache from docker image #158

Merged
merged 2 commits into from
Feb 11, 2025
Merged

fix: Remove uv cache from docker image #158

merged 2 commits into from
Feb 11, 2025

Conversation

daha
Copy link
Contributor

@daha daha commented Jan 24, 2025

This makes the docker image about 25% smaller (~800MB).

Before the change the image is about 3.08GB, reported by docker images and with the change is is about 2.27GB.

Fixes #157

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 24, 2025 •
edited
Loading

Unit Test Results (build & test)

104 tests  ±0   104 ✅ ±0   15s ⏱️ ±0s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 8de0018. ± Comparison against base commit eb2476a.

♻️ This comment has been updated with latest results.

@daha daha mentioned this pull request Jan 24, 2025
Closed
@hsheth2
Copy link
Contributor

hsheth2 commented Jan 27, 2025

Looks like we've got some issues with our CI that should be fixed by #159. Once that's merged, I'll resync this branch with master and hopefully CI will turn green.

@daha
Copy link
Contributor Author

daha commented Jan 28, 2025

It looks like the there are still build issues, build script has problems with the new line between the tags in ${{ steps.docker_meta_slim.outputs.tags }} and the second tag are run as a script...

@daha daha force-pushed the remove-uv-cache-from-docker-image branch 2 times, most recently from e541f71 to 765abba Compare January 31, 2025 09:18
@daha
fix: Remove uv cache from docker image
c311894 
This makes the docker image about 25% smaller (~800MB).
@daha daha force-pushed the remove-uv-cache-from-docker-image branch from 765abba to 13d39df Compare February 10, 2025 19:03
@codecov Codecov
Copy link

codecov bot commented Feb 10, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 44.22%. Comparing base (eb2476a) to head (8de0018).
Report is 1 commits behind head on main.

Additional details and impacted files

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb2476a...8de0018. Read the comment docs.

@daha daha marked this pull request as draft February 10, 2025 21:03
@daha daha force-pushed the remove-uv-cache-from-docker-image branch 2 times, most recently from c9965bd to 6c3f04e Compare February 11, 2025 06:10
@daha
Fix: replace new line with space to run only one command
8de0018 
The new lines are causing problem in `docker image
save` where the second tags run on a separate line
and that command does not exist.
@daha daha force-pushed the remove-uv-cache-from-docker-image branch from 6c3f04e to 8de0018 Compare February 11, 2025 18:07
@daha
Copy link
Contributor Author

daha commented Feb 11, 2025

Yet another update. I trying to fix the problems with the docker image save command or more precisely the the output from ${{ steps.docker_meta_slim.outputs.tags }} which contains new line characters.

@daha daha marked this pull request as ready for review February 11, 2025 18:45
@hsheth2 hsheth2 merged commit be1a067 into acryldata:main Feb 11, 2025
11 checks passed
@hsheth2
Copy link
Contributor

hsheth2 commented Feb 11, 2025

@daha thanks for figuring out the CI issue - seemed like a tricky one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hsheth2 hsheth2 hsheth2 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Docker image is bigger than it needs to be
2 participants
@daha @hsheth2