From d64de407f3f144cf1a22ff66b2a9eef7972e661d Mon Sep 17 00:00:00 2001 From: Alex Wolf Date: Mon, 8 Jan 2024 23:24:43 +0100 Subject: [PATCH 1/4] =?UTF-8?q?=F0=9F=9A=B8=20Allow=20loading=20another=20?= =?UTF-8?q?instance=20in=20the=20same=20Python=20session?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lamindb_setup/_load_instance.py | 16 +++------------- lamindb_setup/dev/django.py | 24 +++++++++++++++--------- 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/lamindb_setup/_load_instance.py b/lamindb_setup/_load_instance.py index d1a0822e4..bdfc5acf8 100644 --- a/lamindb_setup/_load_instance.py +++ b/lamindb_setup/_load_instance.py @@ -95,23 +95,13 @@ 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 and f"{owner}/{name}" != settings.instance.identifier: + close_instance(mute=True) settings_file = instance_settings_file(name, owner) diff --git a/lamindb_setup/dev/django.py b/lamindb_setup/dev/django.py index 415d74489..46726e93d 100644 --- a/lamindb_setup/dev/django.py +++ b/lamindb_setup/dev/django.py @@ -100,13 +100,13 @@ def setup_django( 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, } @@ -144,10 +144,16 @@ 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 + else: + from django.db import connections + + # compare this with add_db_connection in lamindb._registry + 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 + BaseDatabaseWrapper.close_if_health_check_failed = close_if_health_check_failed if configure_only: return None From 7caa7b364b934f6e6cfb000dd11b61854172f7f9 Mon Sep 17 00:00:00 2001 From: Alex Wolf Date: Tue, 9 Jan 2024 00:08:41 +0100 Subject: [PATCH 2/4] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Fix=20and=20improve?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/prod-only/test-load-lock.ipynb | 10 ++-- docs/prod-staging/test-multi-session.ipynb | 53 ++++++++++++++++++---- lamindb_setup/_close.py | 1 + lamindb_setup/_init_instance.py | 21 +++------ lamindb_setup/_load_instance.py | 8 +++- lamindb_setup/dev/django.py | 3 +- 6 files changed, 65 insertions(+), 31 deletions(-) diff --git a/docs/prod-only/test-load-lock.ipynb b/docs/prod-only/test-load-lock.ipynb index 7098815e3..b17f23cc1 100644 --- a/docs/prod-only/test-load-lock.ipynb +++ b/docs/prod-only/test-load-lock.ipynb @@ -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" ] @@ -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", diff --git a/docs/prod-staging/test-multi-session.ipynb b/docs/prod-staging/test-multi-session.ipynb index f1ae46883..ca2fd228d 100644 --- a/docs/prod-staging/test-multi-session.ipynb +++ b/docs/prod-staging/test-multi-session.ipynb @@ -25,7 +25,8 @@ "outputs": [], "source": [ "import lamindb_setup as ln_setup\n", - "import pytest" + "import pytest\n", + "from pathlib import Path" ] }, { @@ -113,14 +114,46 @@ "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.init(storage=\"./testsetup2\")" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "ln_setup.settings.instance.db" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "ln_setup.settings.instance.identifier == \"testuser2/testsetup2\"\n", + "assert ln_setup.settings.instance.storage.is_cloud == False\n", + "assert ln_setup.settings.instance.owner == ln_setup.settings.user.handle\n", + "assert ln_setup.settings.instance.name == \"testsetup2\"\n", + "assert (\n", + " ln_setup.settings.storage.root.as_posix() == Path(\"testsetup2\").resolve().as_posix()\n", + ")\n", + "assert ln_setup.settings.storage.id is not None\n", + "assert ln_setup.settings.instance._sqlite_file.exists()\n", + "assert (\n", + " ln_setup.settings.instance.db\n", + " == f\"sqlite:///{Path('./testsetup2').resolve().as_posix()}/{ln_setup.settings.instance.id.hex}.lndb\"\n", + ")" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "ln_setup.load(\"testuser1/testsetup\")" ] }, { @@ -151,7 +184,7 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.9.16" + "version": "3.10.13" } }, "nbformat": 4, diff --git a/lamindb_setup/_close.py b/lamindb_setup/_close.py index 8ebd4873c..e6c213a83 100644 --- a/lamindb_setup/_close.py +++ b/lamindb_setup/_close.py @@ -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: diff --git a/lamindb_setup/_init_instance.py b/lamindb_setup/_init_instance.py index 4a262eec4..971b82c16 100644 --- a/lamindb_setup/_init_instance.py +++ b/lamindb_setup/_init_instance.py @@ -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 = """ @@ -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 @@ -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) diff --git a/lamindb_setup/_load_instance.py b/lamindb_setup/_load_instance.py index bdfc5acf8..1b481443d 100644 --- a/lamindb_setup/_load_instance.py +++ b/lamindb_setup/_load_instance.py @@ -100,8 +100,12 @@ def load( owner, name = get_owner_name_from_identifier(identifier) # compare normalized identifier with a potentially previously loaded identifier - if settings._instance_exists and f"{owner}/{name}" != settings.instance.identifier: - close_instance(mute=True) + 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) diff --git a/lamindb_setup/dev/django.py b/lamindb_setup/dev/django.py index 46726e93d..796640aa2 100644 --- a/lamindb_setup/dev/django.py +++ b/lamindb_setup/dev/django.py @@ -182,7 +182,7 @@ def setup_django( else: if init: # create migrations - call_command("migrate", verbosity=0) + call_command("migrate", verbosity=2) else: status, latest_migrs = get_migrations_to_sync() if status == "synced": @@ -206,6 +206,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 From d99635c14c7c675a0e4085208e1bb7ea4d05f997 Mon Sep 17 00:00:00 2001 From: Alex Wolf Date: Tue, 9 Jan 2024 01:24:10 +0100 Subject: [PATCH 3/4] =?UTF-8?q?=F0=9F=92=9A=20Fix?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/prod-staging/test-multi-session.ipynb | 31 +++++----------------- lamindb_setup/dev/django.py | 11 +++++--- tests/prod-staging/test_load_instance.py | 16 +++++------ 3 files changed, 21 insertions(+), 37 deletions(-) diff --git a/docs/prod-staging/test-multi-session.ipynb b/docs/prod-staging/test-multi-session.ipynb index ca2fd228d..8a0cc2570 100644 --- a/docs/prod-staging/test-multi-session.ipynb +++ b/docs/prod-staging/test-multi-session.ipynb @@ -114,7 +114,7 @@ "metadata": {}, "outputs": [], "source": [ - "ln_setup.init(storage=\"./testsetup2\")" + "ln_setup.load(\"testuser1/testsetup-prepare\")" ] }, { @@ -123,39 +123,20 @@ "metadata": {}, "outputs": [], "source": [ - "ln_setup.settings.instance.db" - ] - }, - { - "cell_type": "code", - "execution_count": null, - "metadata": {}, - "outputs": [], - "source": [ - "ln_setup.settings.instance.identifier == \"testuser2/testsetup2\"\n", + "ln_setup.settings.instance.identifier == \"testuser1/testsetup-prepare\"\n", "assert ln_setup.settings.instance.storage.is_cloud == False\n", - "assert ln_setup.settings.instance.owner == ln_setup.settings.user.handle\n", - "assert ln_setup.settings.instance.name == \"testsetup2\"\n", + "assert ln_setup.settings.instance.name == \"testsetup-prepare\"\n", "assert (\n", - " ln_setup.settings.storage.root.as_posix() == Path(\"testsetup2\").resolve().as_posix()\n", + " ln_setup.settings.storage.root.as_posix()\n", + " == Path(\"testsetup-prepare\").resolve().as_posix()\n", ")\n", - "assert ln_setup.settings.storage.id is not None\n", "assert ln_setup.settings.instance._sqlite_file.exists()\n", "assert (\n", " ln_setup.settings.instance.db\n", - " == f\"sqlite:///{Path('./testsetup2').resolve().as_posix()}/{ln_setup.settings.instance.id.hex}.lndb\"\n", + " == f\"sqlite:///{Path('./testsetup-prepare').resolve().as_posix()}/{ln_setup.settings.instance.id.hex}.lndb\"\n", ")" ] }, - { - "cell_type": "code", - "execution_count": null, - "metadata": {}, - "outputs": [], - "source": [ - "ln_setup.load(\"testuser1/testsetup\")" - ] - }, { "cell_type": "code", "execution_count": null, diff --git a/lamindb_setup/dev/django.py b/lamindb_setup/dev/django.py index 796640aa2..11bd1a2a3 100644 --- a/lamindb_setup/dev/django.py +++ b/lamindb_setup/dev/django.py @@ -96,6 +96,7 @@ 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 @@ -144,11 +145,10 @@ def setup_django( ) settings.configure(**kwargs) django.setup(set_prefix=False) - else: - from django.db import connections - # compare this with add_db_connection in lamindb._registry - connections.settings["default"] = default_db + # 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 @@ -182,6 +182,9 @@ def setup_django( else: if init: # create migrations + from django.db.migrations.executor import MigrationExecutor + + MigrationExecutor(connections["default"]).loader.applied_migrations = {} call_command("migrate", verbosity=2) else: status, latest_migrs = get_migrations_to_sync() diff --git a/tests/prod-staging/test_load_instance.py b/tests/prod-staging/test_load_instance.py index 921c96e2d..3a60dce28 100644 --- a/tests/prod-staging/test_load_instance.py +++ b/tests/prod-staging/test_load_instance.py @@ -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 @@ -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", @@ -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( @@ -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" From 877035c2ff1c0b74258671bb67974ae7abd9476f Mon Sep 17 00:00:00 2001 From: Alex Wolf Date: Tue, 9 Jan 2024 01:27:26 +0100 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=94=87=20Fix=20verbosity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lamindb_setup/dev/django.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lamindb_setup/dev/django.py b/lamindb_setup/dev/django.py index 11bd1a2a3..7889d13e7 100644 --- a/lamindb_setup/dev/django.py +++ b/lamindb_setup/dev/django.py @@ -181,11 +181,7 @@ def setup_django( isettings._update_cloud_sqlite_file(unlock_cloud_sqlite=False) else: if init: - # create migrations - from django.db.migrations.executor import MigrationExecutor - - MigrationExecutor(connections["default"]).loader.applied_migrations = {} - call_command("migrate", verbosity=2) + call_command("migrate", verbosity=0) else: status, latest_migrs = get_migrations_to_sync() if status == "synced":