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

adds zarf cache flag #446

Merged
merged 6 commits into from
Apr 21, 2022
Merged

adds zarf cache flag #446

merged 6 commits into from
Apr 21, 2022

Conversation

UncleGedd
Copy link
Contributor

@UncleGedd UncleGedd commented Apr 15, 2022

Description

Adds a zarf-cache flag to specify where images are stored on a user's machine.

Related Issue

Fixes issue #402

Type of change

  • New feature (non-breaking change which adds functionality)

@UncleGedd UncleGedd self-assigned this Apr 15, 2022
@UncleGedd UncleGedd force-pushed the add-image-cache-flag branch 3 times, most recently from efe2a22 to 8a6d325 Compare April 15, 2022 18:46
@UncleGedd
Copy link
Contributor Author

Currently, the Zarf image cache resides in the home directory and this PR adds the option to put the cache in a subpath within the home directory. Would we like the option to put the image cache outside of the home directory?

@UncleGedd UncleGedd requested a review from YrrepNoj April 15, 2022 18:56
@UncleGedd UncleGedd force-pushed the add-image-cache-flag branch from 8a6d325 to d618d1d Compare April 15, 2022 19:54
@RothAndrew
Copy link
Contributor

RothAndrew commented Apr 15, 2022

@YrrepNoj you were working with someone on something kinda-adjacent to this kind of thing right? IIRC they had very little space on their root drive (where the home directory is mounted) but they had a separate filesystem mount that had tons of space.

Given that, perhaps the user should be able to specify anywhere they want, rather than just somewhere inside their home directory.

@UncleGedd UncleGedd force-pushed the add-image-cache-flag branch from d618d1d to 6783714 Compare April 15, 2022 21:56
@UncleGedd UncleGedd changed the title WIP: adds zarf cache flag adds zarf cache flag Apr 15, 2022
@jeff-mccoy jeff-mccoy added packager needs-tests PR Label - Tests required to merge enhancement ✨ New feature or request labels Apr 17, 2022
src/cmd/root.go Outdated Show resolved Hide resolved
@jeff-mccoy
Copy link
Contributor

Thanks @UncleGedd, this is a helpful PR. I think we should discuss moving the flag down to zarf package create though as mentioned by @YrrepNoj.

@UncleGedd
Copy link
Contributor Author

Thanks @UncleGedd, this is a helpful PR. I think we should discuss moving the flag down to zarf package create though as mentioned by @YrrepNoj.

Easy fix! Will update and add an e2e test for zarf package create

@UncleGedd UncleGedd force-pushed the add-image-cache-flag branch 2 times, most recently from 018d0e1 to 897cbe8 Compare April 18, 2022 23:22
@UncleGedd UncleGedd force-pushed the add-image-cache-flag branch from 897cbe8 to ca1b754 Compare April 19, 2022 13:27
Copy link
Contributor

@jeff-mccoy jeff-mccoy left a comment

Choose a reason for hiding this comment

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

Overall looks good and is a valuable addition, only comment is whether the regex for path detection is the best way to do that vs some path function.

@jeff-mccoy jeff-mccoy removed the needs-tests PR Label - Tests required to merge label Apr 20, 2022
@jeff-mccoy jeff-mccoy merged commit 5a38917 into master Apr 21, 2022
@jeff-mccoy jeff-mccoy deleted the add-image-cache-flag branch April 21, 2022 04:14
Noxsios pushed a commit that referenced this pull request Mar 8, 2023
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.

4 participants