Skip to content

feat: Implement a --use-cache flag closes #100#104

Merged
Pavel Zwerschke (pavelzw) merged 6 commits intoQuantco:mainfrom
awesomebytes:awesomebytes/add_cache
Feb 23, 2025
Merged

feat: Implement a --use-cache flag closes #100#104
Pavel Zwerschke (pavelzw) merged 6 commits intoQuantco:mainfrom
awesomebytes:awesomebytes/add_cache

Conversation

@awesomebytes
Copy link
Copy Markdown
Contributor

@awesomebytes Sam Pfeiffer (awesomebytes) commented Feb 23, 2025

Motivation

Following up on #100 (thanks for the opportunity Pavel Zwerschke (@pavelzw) !).

As the README of this PR states, having a cache for downloads is useful when:

  • Creating multiple packs with overlapping dependencies
  • Working with large packages that take time to download
  • Operating in environments with limited bandwidth
  • Running CI/CD pipelines where package caching can significantly improve build times

pixi's cache is for already-extracted packages, but here we need them un-extracted, so implementing an optional (behind the --use-cache flag) cache for downloads.

I have little experience in Rust[1], let me know if this is an okay implementation.

There are some formatting changes, I used the autoformat feature from VSCode with the rust-analyzer extension.

Changes

  • Added a --use-cache CACHE_DIR flag.
  • Added implementation for it in the download function.
  • Added a test for it.
  • Added README docs about it.

[1] It's so easy to jump into a project thanks to cargo! I just cloned, ran cargo build, then cargo test (which failed due to my slow internet)... and then I could just cargo run to test my changes (before I added my test, which I should probably have done first).

closes #100

@awesomebytes Sam Pfeiffer (awesomebytes) changed the title Implement a --use-cache flag closes #100 feat: Implement a --use-cache flag closes #100 Feb 23, 2025
@awesomebytes
Copy link
Copy Markdown
Contributor Author

Note that for the integration tests, if you wish, cache could be used. Particularly:

  • test_reproducible_shasum does two pack operations in a row

Or, having a global cache for all the tests, would accelerate them all anywhere where the internet is not super fast (and running the tests will hit the hosting servers less). But this may mess with expectations of the tests or their particular goals. I did not want to implement that in case it was too controversial.

@pavelzw
Copy link
Copy Markdown
Member

Note that for the integration tests, if you wish, cache could be used

i don't think this is necessary. but as mentioned in #104 (comment), i think it would be nice to assert that the SHA stays the same even with cache

@awesomebytes
Copy link
Copy Markdown
Contributor Author

Oh, also, there's no short-version of the --use-cache flag, that may be a taste thing. I think it would default to -u which feels awkward. And -c doesn't convince me either (--create-executable doesn't have one, and would default to c?).

So, yeah, if it should have a short-version, you should probably be the ones to decide which :)

Also, I chose to make --use-cache USER_PROVIDED_DIR over --use-cache without any arguments as I would most probably see the need to be able to specify the location anyways (e.g. /tmp /shm /some_shared_folder).

@pavelzw
Copy link
Copy Markdown
Member

So, yeah, if it should have a short-version, you should probably be the ones to decide which

i think only having a long version should be fine here

I chose to make --use-cache USER_PROVIDED_DIR over --use-cache without any arguments

sounds sensible 👍🏻

@awesomebytes
Copy link
Copy Markdown
Contributor Author

Pavel Zwerschke (@pavelzw) all ready, I think :)

Copy link
Copy Markdown
Member

@delsner Daniel Elsner (delsner) left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for implementing this so quickly!

@awesomebytes
Copy link
Copy Markdown
Contributor Author

Thank you for the tool and the opportunity!

Any comment in regards of the code/tests? For me to learn, basically

Copy link
Copy Markdown
Member

@pavelzw Pavel Zwerschke (pavelzw) left a comment

Choose a reason for hiding this comment

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

thanks, code looks good to me!

@pavelzw
Copy link
Copy Markdown
Member

failing ci is intended as --create-executable doesn't work with a non-existent release

@pavelzw Pavel Zwerschke (pavelzw) merged commit 22bf0c6 into Quantco:main Feb 23, 2025
12 of 16 checks passed
@pavelzw Pavel Zwerschke (pavelzw) added the enhancement New feature or request label Feb 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pixi-pack re-downloads packages in a pixi project that just downloaded them

3 participants