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

🚸 Allow loading another instance in the same Python session #612

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions docs/prod-only/test-load-lock.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,9 @@
"source": [
"# test ignore_prev_locker=True in unlock_cloud_sqlite_upon_exception\n",
"# i.e. test that the locker doesn't unlock if the locker hasn't changed during the function execution\n",
"with pytest.raises(RuntimeError):\n",
" ln_setup.load(\"lndb-load-test-lock\")\n",
"# this won't raise a runtime error anymore because multiple instance loading is now permitted\n",
"# with pytest.raises(RuntimeError):\n",
"# ln_setup.load(\"lndb-load-test-lock\")\n",
"\n",
"assert ln_setup.settings.instance._cloud_sqlite_locker._has_lock"
]
Expand All @@ -221,8 +222,9 @@
"# purely technical varibale to test failed load after locking\n",
"ln_setup._load_instance._TEST_FAILED_LOAD = True\n",
"\n",
"with pytest.raises(RuntimeError):\n",
" ln_setup.load(\"lndb-load-test-lock\")\n",
"# this won't raise a runtime error anymore because multiple instance loading is now permitted\n",
"# with pytest.raises(RuntimeError):\n",
"# ln_setup.load(\"lndb-load-test-lock\")\n",
"\n",
"assert ln_setup.dev.cloud_sqlite_locker._locker._has_lock is None\n",
"\n",
Expand Down
34 changes: 24 additions & 10 deletions docs/prod-staging/test-multi-session.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
"outputs": [],
"source": [
"import lamindb_setup as ln_setup\n",
"import pytest"
"import pytest\n",
"from pathlib import Path"
]
},
{
Expand Down Expand Up @@ -113,14 +114,27 @@
"metadata": {},
"outputs": [],
"source": [
"with pytest.raises(RuntimeError):\n",
" ln_setup.init(storage=\"./testsetup2\")\n",
"with pytest.raises(RuntimeError):\n",
" ln_setup.load(\"testsetup\")\n",
"with pytest.raises(RuntimeError):\n",
" ln_setup.migrate.create()\n",
"with pytest.raises(RuntimeError):\n",
" ln_setup.migrate.deploy()"
"ln_setup.load(\"testuser1/testsetup-prepare\")"
]
},
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"ln_setup.settings.instance.identifier == \"testuser1/testsetup-prepare\"\n",
"assert ln_setup.settings.instance.storage.is_cloud == False\n",
"assert ln_setup.settings.instance.name == \"testsetup-prepare\"\n",
"assert (\n",
" ln_setup.settings.storage.root.as_posix()\n",
" == Path(\"testsetup-prepare\").resolve().as_posix()\n",
")\n",
"assert ln_setup.settings.instance._sqlite_file.exists()\n",
"assert (\n",
" ln_setup.settings.instance.db\n",
" == f\"sqlite:///{Path('./testsetup-prepare').resolve().as_posix()}/{ln_setup.settings.instance.id.hex}.lndb\"\n",
")"
]
},
{
Expand Down Expand Up @@ -151,7 +165,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.9.16"
"version": "3.10.13"
}
},
"nbformat": 4,
Expand Down
1 change: 1 addition & 0 deletions lamindb_setup/_close.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ def close(mute: bool = False) -> None:
current_instance_settings_file().unlink()
delete_bionty_sources_yaml()
clear_locker()
settings._instance_settings = None
if not mute:
logger.success(f"closed instance: {instance}")
else:
Expand Down
21 changes: 7 additions & 14 deletions lamindb_setup/_init_instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,10 @@ def reload_lamindb(isettings: InstanceSettings):
import lamindb

importlib.reload(lamindb)
logger.important(f"lamindb instance: {isettings.identifier}")
else:
# only log if we're outside lamindb
# lamindb itself logs upon import!
logger.important(f"loaded instance: {isettings.owner}/{isettings.name}")
verbosity = logger._verbosity
logger.set_verbosity(2)
logger.success(f"loaded instance: {isettings.identifier}")
logger.set_verbosity(verbosity)


ERROR_SQLITE_CACHE = """
Expand Down Expand Up @@ -137,15 +136,6 @@ def init(
db: {}
schema: {}
"""
from ._check_instance_setup import check_instance_setup

if check_instance_setup() and not _test:
raise RuntimeError(
"Currently don't support init or load of multiple instances in the same"
" Python session. We will bring this feature back at some point."
)
else:
close_instance(mute=True)
# clean up in next refactor
# avoid circular import
from ._load_instance import load
Expand All @@ -167,6 +157,9 @@ def init(
assert settings.user.uid # check user is logged in
owner = settings.user.handle

if settings._instance_exists:
close_instance()

schema = validate_schema_arg(schema)
validate_storage_root_arg(str(storage))
validate_db_arg(db)
Expand Down
20 changes: 7 additions & 13 deletions lamindb_setup/_load_instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,23 +95,17 @@ def load(
storage: `Optional[PathLike] = None` - Load the instance with an
updated default storage.
"""
from ._check_instance_setup import check_instance_setup
from .dev._hub_core import load_instance as load_instance_from_hub

owner, name = get_owner_name_from_identifier(identifier)

if check_instance_setup() and not _test:
raise RuntimeError(
"Currently don't support init or load of multiple instances in the same"
" Python session. We will bring this feature back at some point."
)
else:
# compare normalized identifier with a potentially previously loaded identifier
if (
settings._instance_exists
and f"{owner}/{name}" != settings.instance.identifier
):
close_instance(mute=True)
# compare normalized identifier with a potentially previously loaded identifier
if settings._instance_exists:
if f"{owner}/{name}" == settings.instance.identifier:
logger.important(f"instance already loaded: {settings.instance.identifier}")
return None
else:
close_instance()

settings_file = instance_settings_file(name, owner)

Expand Down
26 changes: 16 additions & 10 deletions lamindb_setup/dev/django.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,17 +96,18 @@ def setup_django(

import dj_database_url
import django
from django.db import connections
from django.conf import settings
from django.core.management import call_command

# configuration
default_db = dj_database_url.config(
default=isettings.db,
# see comment next to patching BaseDatabaseWrapper below
conn_max_age=CONN_MAX_AGE,
conn_health_checks=True,
)
if not settings.configured:
default_db = dj_database_url.config(
default=isettings.db,
# see comment next to patching BaseDatabaseWrapper below
conn_max_age=CONN_MAX_AGE,
conn_health_checks=True,
)
DATABASES = {
"default": default_db,
}
Expand Down Expand Up @@ -144,10 +145,15 @@ def setup_django(
)
settings.configure(**kwargs)
django.setup(set_prefix=False)
# https://laminlabs.slack.com/archives/C04FPE8V01W/p1698239551460289
from django.db.backends.base.base import BaseDatabaseWrapper

BaseDatabaseWrapper.close_if_health_check_failed = close_if_health_check_failed
# compare this with add_db_connection in lamindb._registry
# is used to update the default connection string
connections.settings["default"] = default_db

# https://laminlabs.slack.com/archives/C04FPE8V01W/p1698239551460289
from django.db.backends.base.base import BaseDatabaseWrapper

BaseDatabaseWrapper.close_if_health_check_failed = close_if_health_check_failed

if configure_only:
return None
Expand Down Expand Up @@ -175,7 +181,6 @@ def setup_django(
isettings._update_cloud_sqlite_file(unlock_cloud_sqlite=False)
else:
if init:
# create migrations
call_command("migrate", verbosity=0)
else:
status, latest_migrs = get_migrations_to_sync()
Expand All @@ -200,6 +205,7 @@ def setup_django(
current_instance_settings_file().unlink()
if current_settings_file_existed:
shutil.copy(current_settings_file.with_name("_tmp.env"), current_settings_file)
current_settings_file.with_name("_tmp.env").unlink()

global IS_SETUP
IS_SETUP = True
16 changes: 8 additions & 8 deletions tests/prod-staging/test_load_instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
import pytest
from laminhub_rest.routers.collaborator import delete_collaborator
from laminhub_rest.routers.instance import add_collaborator

from lamindb_setup.dev._hub_core import load_instance as load_instance_from_hub
import lamindb_setup as ln_setup
from lamindb_setup._load_instance import get_owner_name_from_identifier
from lamindb_setup.dev._hub_client import connect_hub_with_auth
from lamindb_setup.dev._hub_crud import sb_update_instance

Expand All @@ -26,9 +27,7 @@ def test_load_remote_instance():
def test_load_after_revoked_access():
# can't currently test this on staging as I'm missing the accounts
if os.getenv("LAMIN_ENV") == "prod":
ln_setup.login(
"static-testuser1@lamin.ai", password="static-testuser1-password"
)
ln_setup.login("static-testuser1@lamin.ai", key="static-testuser1-password")
admin_token = ln_setup.settings.user.access_token
add_collaborator(
"static-testuser2",
Expand All @@ -37,9 +36,7 @@ def test_load_after_revoked_access():
"write",
f"Bearer {admin_token}",
)
ln_setup.login(
"static-testuser2@lamin.ai", password="static-testuser2-password"
)
ln_setup.login("static-testuser2@lamin.ai", key="static-testuser2-password")
ln_setup.load("https://lamin.ai/laminlabs/static-testinstance1", _test=True)
assert ln_setup.settings.instance.storage.root_as_str == "s3://lndb-setup-ci"
delete_collaborator(
Expand All @@ -49,7 +46,10 @@ def test_load_after_revoked_access():
f"Bearer {admin_token}",
)
with pytest.raises(RuntimeError) as error:
ln_setup.load("https://lamin.ai/laminlabs/static-testinstance1", _test=True)
owner, name = get_owner_name_from_identifier(
"laminlabs/static-testinstance1"
)
load_instance_from_hub(owner=owner, name=name)
assert (
error.exconly()
== "RuntimeError: Instance laminlabs/static-testinstance1 not"
Expand Down
Loading