-
Notifications
You must be signed in to change notification settings - Fork 238
Feat: Add support for configurable cache directory #4869
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
Conversation
tests/core/test_context.py
Outdated
) | ||
), | ||
model_defaults=model_defaults, | ||
cache_dir="{custom_cache_dir}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think injecting the path as a quoted string here is breaking the tests on Windows, because it ends up being cache_dir="C:\some\path"
and the \ is treated as an escape
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks you’re right updated the test to use as posix to have forward slashes in all platforms
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, this will definitely help in Notebook environments (Fabric in particular) which limit the number of files in the persistent storage.
I notice that cache_dir
vs cache_path
is used a bit inconsistently though? Should it be one or the other?
sqlmesh/core/config/scheduler.py
Outdated
@@ -105,7 +105,7 @@ def create_state_sync(self, context: GenericContext) -> StateSync: | |||
|
|||
schema = context.config.get_state_schema(context.gateway) | |||
return EngineAdapterStateSync( | |||
engine_adapter, schema=schema, context_path=context.path, console=context.console | |||
engine_adapter, schema=schema, context_path=context.cache_dir, console=context.console |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the context_path
parameter be renamed to cache_dir
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes good point renamed to use the cache_dir
name here and everywhere else that I was using cache_path for consistency
5c35a90
to
cca8703
Compare
cca8703
to
6bd9463
Compare
By default SQLMesh stores its cache in a
.cache
folder inside the project directory. This introduces a configuration option to specify a custom cache location, either as a relative path or an absolute path. Fixes: #4307