kedro-viz
kedro-viz copied to clipboard
Removing `SESSION_STORE_ARGS` default from starters
Context
Related:
- [x] https://github.com/kedro-org/kedro-viz/issues/1891
- [ ] https://github.com/kedro-org/kedro-viz/pull/1915
As Kedro-viz is introducing a default "SESSION_STORE_ARGS", we should moving away from setting SESSION_STORE_ARGS in kedro-starters and leave that for kedro-viz. Noted this is necessary as we currently have a different default:
SESSION_STORE_ARGS = {"path": str(Path(__file__).parents[2] / )}
but we want to change to
SESSION_STORE_ARGS = {"path": str(Path(__file__).parents[2] / .viz)}
Development Notes:
We need to upgrade the lower pin of kedro-viz to 9.2.0 to make sure the default will be .viz as well.
@ravi-kumar-pilla @rashidakanchwala Can we make it so that Kedro-Viz assumes a default
SESSION_STORE_ARGSofSESSION_STORE_ARGS = {"path": str(Path(__file__).parents[2] / ".viz")}?
That would be the first step towards fixing the issue. The second one would be to update the starters to actually remove the
SESSION_STORE_ARGSfrom them, so that Viz uses its default.We are also overloading "SESSION_STORE_ARGS" as "VIZ_METADATA_ARGS" here, the stats.json has nothing to do with session store.
@noklam Could you open a new issue about this describing it in a bit more detail?
Originally posted by @astrojuanlu in https://github.com/kedro-org/kedro-viz/issues/1891#issuecomment-2132963984
@noklam, @astrojuanlu , @ravi-kumar-pilla -
- I was thinking we either add this to settings.py for starters
# Setup for collaborative experiment tracking.
# The SQLite DB required for experiment tracking is stored by default in the .viz folder of your project.
# To store it in another directory, provide the path: SESSION_STORE_ARGS = {"path": "<Your PATH>"}
- or, we could omit Session Store setup entirely from settings.py and include all the details in the documentation.
@noklam, @astrojuanlu , @ravi-kumar-pilla -
- I was thinking we either add this to settings.py for starters
# Setup for collaborative experiment tracking. # The SQLite DB required for experiment tracking is stored by default in the .viz folder of your project. # To store it in another directory, provide the path: SESSION_STORE_ARGS = {"path": "<Your PATH>"}
Just to make sure we are on the same page, the PR only provides a default session store db path and stats path. The users still need to provide SESSION_STORE_CLASS in settings.py to customize how session data is stored. So we cannot omit the entire Session Store setup. i.e.,
from kedro_viz.integrations.kedro.sqlite_store import SQLiteStore
SESSION_STORE_CLASS = SQLiteStore
So, I think it is better we add comments in settings.py as you suggested in (1 - # Setup for experiment tracking). We can also document this in the project settings doc. Let me know your thoughts.
Thank you
The users still need to provide
SESSION_STORE_CLASSinsettings.pyto customize how session data is stored.
Can't we also provide a default for that one? In the same way we don't have to make users declare their DATA_CATALOG_CLASS for instance
The users still need to provide
SESSION_STORE_CLASSinsettings.pyto customize how session data is stored.Can't we also provide a default for that one? In the same way we don't have to make users declare their
DATA_CATALOG_CLASSfor instance
I am not sure if that is something we can do from viz. So the changes to move SESSION_STORE_ARGS are made in this class which is assigned in settings.py as SESSION_STORE_CLASS. This gets initialized in _init_store of KedroSession.
Thank you
I think SESSION_STORE_CLASS is still needed. SESSION_STORE_CLASS_ARGS can be left as a comment for discoverability
Hi Team,
Should I update the settings.py in each starter with the below block ?
Before -
# Keyword arguments to pass to the `SESSION_STORE_CLASS` constructor.
# SESSION_STORE_ARGS = {
# "path": "./sessions"
# }
After -
# Setup for Experiment Tracking
# The SQLite DB required for experiment tracking is stored by default in the .viz folder of your project.
# To store it in another directory, provide the keyword argument `SESSION_STORE_ARGS` to pass to the `SESSION_STORE_CLASS` constructor.
# SESSION_STORE_ARGS = {"path": str(Path(__file__).parents[2] / .viz)}
Since this is a repeat block for all the starters, I wanted to get your feedback before making changes in all the starters. @astrojuanlu @noklam @rashidakanchwala
Thank you
LGTM 👍🏼
LGTM. thanks