Move to using cache directory in KITOPS_HOME instead of the system temp dir#762
Merged
amisevsk merged 3 commits intokitops-ml:mainfrom Feb 13, 2025
Merged
Move to using cache directory in KITOPS_HOME instead of the system temp dir#762amisevsk merged 3 commits intokitops-ml:mainfrom
amisevsk merged 3 commits intokitops-ml:mainfrom
Conversation
To enable using Kit in a containerized environment, avoid using the system-wide temporary directory. Instead, use $KITOPS_HOME/cache for temporary files in import, pack, etc. This allows for mounting a volume to $KITOPS_HOME/cache to add storage to a container. Some functions, such as unpacking the dev mode harness and setting up tests still use the default system temporary directory, to ensure those files are cleaned up.
amisevsk
force-pushed
the
cache-directory
branch
from
593e93d to
4227edf
Compare
gorkem
approved these changes
Feb 12, 2025
| import ( | ||
| "fmt" | ||
| "io" | ||
| "kitops/pkg/lib/filesystem/cache" |
Member
There was a problem hiding this comment.
Should this be aliased like fsCache not to be confused with the cache in here.
Contributor
Author
There was a problem hiding this comment.
Added alias and also renamed the cmd package to kitcache (like we have with kitimport and kitinit`)
gorkem
approved these changes
Feb 12, 2025
amisevsk
force-pushed
the
cache-directory
branch
from
4227edf to
c2a6be1
Compare
Since cancelling a Kit command with SIGINT can leave files in the cache, and since we can no longer rely on the system to clean these files up eventually, we'll need to do something similar to what we do for kit pull: when a command completes successfully, assume that that command's cache directory should be empty and so remove any files present. This should clean up files left over after previous cancelled runs while not impacting any one action.
amisevsk
force-pushed
the
cache-directory
branch
from
c2a6be1 to
3bb78ab
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Stop using the system's normal temporary directory for temp files, and instead use
$KITOPS_HOME/cache. This allows more easily mounting storage to e.g. containers to support large cached files. However, since this directory will not be emptied by the system, we have to clean it up.To manage automatic cleanup (in case a command is cancelled via SIGINT, for example), we do something similar to how we handle the ingest direction for kit pull: if a command completes successfully, we'll clear its corresponding cache subdir (e.g. a successful import will clear all cached import files). This limits how much data can leak into the cache.
This PR also includes a new command:
kit cache [info|clear]. Theinfosubcommand can be used to see to total size of the cache, if it's not empty. The clear command can be used to manually clean up any stray files.The changes in this PR do not touch
kit pull, since it manages its own ingest directory and I didn't want to risk introducing a subtle bug. However, the current cache implementation should support that too, as a future improvement.Finally, this change opens the door to resumable hugging face downloads, which can be implemented in another PR.
Linked issues
Closes #758