-
Notifications
You must be signed in to change notification settings - Fork 114
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
Feature/remove session store #2219
Changes from 10 commits
11989df
943699d
7cd9f79
cc50ab6
d283fca
e083357
d8abbd3
2fb3576
ab66d10
0490962
57cba0b
3ff2a07
4ad56fa
7bf73d7
ccb3e26
60b4e09
96488b3
cd42a83
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,25 +2,6 @@ | |
# List the installed plugins for which to disable auto-registry | ||
# DISABLE_HOOKS_FOR_PLUGINS = ("kedro-viz",) | ||
|
||
from pathlib import Path | ||
|
||
# Define where to store data from a KedroSession. Defaults to BaseSessionStore. | ||
# from kedro.framework.session.store import ShelveStore | ||
from kedro_viz.integrations.kedro.sqlite_store import SQLiteStore | ||
|
||
SESSION_STORE_CLASS = SQLiteStore | ||
SESSION_STORE_ARGS = {"path": str(Path(__file__).parents[2] / "data")} | ||
|
||
# Setup for collaborative experiment tracking. | ||
# SESSION_STORE_ARGS = {"path": str(Path(__file__).parents[2] / "data"), | ||
# "remote_path": "s3://{path-to-session_store}" } | ||
|
||
# Define custom context class. Defaults to `KedroContext` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can keep the code from line 18-23 as is, as this is not related to ET |
||
# CONTEXT_CLASS = KedroContext | ||
|
||
# Define the configuration folder. Defaults to `conf` | ||
# CONF_ROOT = "conf" | ||
|
||
from kedro.config import OmegaConfigLoader # NOQA | ||
|
||
CONFIG_LOADER_CLASS = OmegaConfigLoader |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,21 +21,3 @@ def configure_wal_for_azure(engine): | |
if is_azure_ml: | ||
with engine.connect() as conn: | ||
conn.execute(text("PRAGMA journal_mode=WAL;")) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file is for handling SQLite which is not needed anymore, so I guess we can delete the file entirely |
||
|
||
def make_db_session_factory(session_store_location: str) -> sessionmaker: | ||
"""SQLAlchemy connection to a SQLite DB""" | ||
database_url = f"sqlite:///{session_store_location}" | ||
engine = create_engine(database_url, connect_args={"check_same_thread": False}) | ||
# TODO: making db session factory shouldn't depend on models. | ||
# So want to move the table creation elsewhere ideally. | ||
# But this means returning engine as well as session class. | ||
|
||
# Check if we are running in an Azure ML environment if so enable WAL mode. | ||
configure_wal_for_azure(engine) | ||
|
||
# Create the database tables if they do not exist. | ||
Base.metadata.create_all(bind=engine) | ||
|
||
# Return a session factory bound to the engine. | ||
return sessionmaker(bind=engine) |
This file was deleted.
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.
We need to make changes to all the starter templates (may be a note saying kedro-viz v11 does not support ET) as a follow up - https://github.com/kedro-org/kedro-starters/blob/main/spaceflights-pandas-viz/%7B%7B%20cookiecutter.repo_name%20%7D%7D/src/%7B%7B%20cookiecutter.python_package%20%7D%7D/settings.py
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.
hey, @merelcht has already raised this concern, and I’ll create a ticket for it. For now, we’ve added a deprecation warning for Kedro datasets to alert users about this change coming in the next release.