Skip to content

Commit

Permalink
🎨 Simplify InstanceNotEmpty UX (#929)
Browse files Browse the repository at this point in the history
* 🎨 Simplify InstanceNotEmpty UX

Signed-off-by: zethson <[email protected]>

* 🎨 Latest Ruff

Signed-off-by: zethson <[email protected]>

---------

Signed-off-by: zethson <[email protected]>
  • Loading branch information
Zethson authored Jan 8, 2025
1 parent ebd93eb commit 58f4749
Show file tree
Hide file tree
Showing 11 changed files with 112 additions and 47 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ repos:
docs/notes/
)
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.5.5
rev: v0.8.6
hooks:
- id: ruff
args: [--fix, --exit-non-zero-on-fix, --unsafe-fixes]
Expand Down
9 changes: 6 additions & 3 deletions docs/hub-cloud/01-init-local-instance.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"from pathlib import Path\n",
"from lamindb.models import Storage\n",
"\n",
"assert ln_setup.settings.instance.storage.type_is_cloud == False\n",
"assert ln_setup.settings.instance.storage.type_is_cloud is False\n",
"assert ln_setup.settings.instance.owner == ln_setup.settings.user.handle\n",
"assert ln_setup.settings.instance.name == \"mydata\"\n",
"assert ln_setup.settings.storage.root.as_posix() == Path(\"mydata\").resolve().as_posix()\n",
Expand All @@ -62,7 +62,10 @@
" == f\"sqlite:///{Path('./mydata').resolve().as_posix()}/{ln_setup.settings.instance._id.hex}.lndb\"\n",
")\n",
"assert ln_setup.settings.storage._instance_id == ln_setup.settings.instance._id\n",
"assert Storage.objects.get(instance_uid=ln_setup.settings.instance.uid).root == ln_setup.settings.storage.root_as_str\n"
"assert (\n",
" Storage.objects.get(instance_uid=ln_setup.settings.instance.uid).root\n",
" == ln_setup.settings.storage.root_as_str\n",
")"
]
},
{
Expand All @@ -84,7 +87,7 @@
"\n",
"settings_file = instance_settings_file(\"mydata\", \"testuser1\")\n",
"assert not storage_root.exists()\n",
"assert settings_file.exists() == False"
"assert settings_file.exists() is False"
]
}
],
Expand Down
2 changes: 1 addition & 1 deletion docs/hub-cloud/02-connect-local-instance.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
"source": [
"from pathlib import Path\n",
"\n",
"assert ln_setup.settings.instance.storage.type_is_cloud == False\n",
"assert ln_setup.settings.instance.storage.type_is_cloud is False\n",
"assert ln_setup.settings.instance.name == \"mydata\"\n",
"\n",
"root_path = Path(\"./mydata\").resolve()\n",
Expand Down
36 changes: 27 additions & 9 deletions docs/hub-cloud/03-add-managed-storage.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,10 @@
"source": [
"with pytest.raises(ValueError) as error:\n",
" set_managed_storage(\"./storage2\")\n",
"assert error.exconly() == \"ValueError: Can't add additional managed storage locations for instances that aren't managed through the hub.\""
"assert (\n",
" error.exconly()\n",
" == \"ValueError: Can't add additional managed storage locations for instances that aren't managed through the hub.\"\n",
")"
]
},
{
Expand Down Expand Up @@ -129,7 +132,9 @@
"source": [
"storage2_uid = ln_setup.settings.storage.uid\n",
"assert ln_setup.settings.storage.root_as_str == f\"{Path.cwd()}/storage2\"\n",
"assert (ln_setup.settings.storage.root / \".lamindb/_is_initialized\").read_text() == ln_setup.settings.storage.uid\n",
"assert (\n",
" ln_setup.settings.storage.root / \".lamindb/_is_initialized\"\n",
").read_text() == ln_setup.settings.storage.uid\n",
"assert ln_setup.settings.storage.is_on_hub"
]
},
Expand Down Expand Up @@ -159,7 +164,9 @@
"outputs": [],
"source": [
"assert ln_setup.settings.storage.root_as_str == f\"{Path.cwd()}/storage1\"\n",
"assert (ln_setup.settings.storage.root / \".lamindb/_is_initialized\").read_text() == ln_setup.settings.storage.uid\n",
"assert (\n",
" ln_setup.settings.storage.root / \".lamindb/_is_initialized\"\n",
").read_text() == ln_setup.settings.storage.uid\n",
"assert ln_setup.settings.storage.is_on_hub\n",
"assert ln_setup.settings.storage.uid == storage1_uid"
]
Expand Down Expand Up @@ -190,7 +197,9 @@
"outputs": [],
"source": [
"assert ln_setup.settings.storage.root_as_str == f\"{Path.cwd()}/storage1\"\n",
"assert (ln_setup.settings.storage.root / \".lamindb/_is_initialized\").read_text() == ln_setup.settings.storage.uid\n",
"assert (\n",
" ln_setup.settings.storage.root / \".lamindb/_is_initialized\"\n",
").read_text() == ln_setup.settings.storage.uid\n",
"assert ln_setup.settings.storage.is_on_hub\n",
"assert ln_setup.settings.storage.uid == storage1_uid"
]
Expand All @@ -214,7 +223,9 @@
"source": [
"storage2_uid = ln_setup.settings.storage.uid\n",
"assert ln_setup.settings.storage.root_as_str == f\"{Path.cwd()}/storage2\"\n",
"assert (ln_setup.settings.storage.root / \".lamindb/_is_initialized\").read_text() == ln_setup.settings.storage.uid\n",
"assert (\n",
" ln_setup.settings.storage.root / \".lamindb/_is_initialized\"\n",
").read_text() == ln_setup.settings.storage.uid\n",
"assert ln_setup.settings.storage.is_on_hub\n",
"assert ln_setup.settings.storage.uid == storage2_uid"
]
Expand Down Expand Up @@ -251,7 +262,9 @@
"assert ln_setup.settings.storage.type_is_cloud\n",
"assert ln_setup.settings.storage.root_as_str == \"s3://lamindb-ci/storage3\"\n",
"assert ln_setup.settings.storage.region == \"us-west-1\"\n",
"assert (ln_setup.settings.storage.root / \".lamindb/_is_initialized\").read_text() == ln_setup.settings.storage.uid\n",
"assert (\n",
" ln_setup.settings.storage.root / \".lamindb/_is_initialized\"\n",
").read_text() == ln_setup.settings.storage.uid\n",
"# root.fs contains the underlying fsspec filesystem\n",
"assert (\n",
" ln_setup.settings.storage.root.fs.cache_regions # set by lamindb to True for s3 by default\n",
Expand Down Expand Up @@ -305,7 +318,9 @@
"source": [
"with pytest.raises(ValueError) as error:\n",
" set_managed_storage(\"gs://rxrx1-europe-west4/images/test/HEPG2-08\")\n",
"assert error.exconly().startswith(\"ValueError: Cannot manage storage without write access\")"
"assert error.exconly().startswith(\n",
" \"ValueError: Cannot manage storage without write access\"\n",
")"
]
},
{
Expand Down Expand Up @@ -335,7 +350,7 @@
" account_id=testuser2.id,\n",
" role=\"write\",\n",
" schema_id=None,\n",
" skip_insert_user_table=True\n",
" skip_insert_user_table=True,\n",
")\n",
"User.objects.create(uid=testuser2.lnid, handle=testuser2.handle, name=testuser2.name)\n",
"admin_hub.auth.close()"
Expand Down Expand Up @@ -418,7 +433,10 @@
"from lamindb_setup.core._hub_client import call_with_fallback_auth\n",
"from lamindb_setup.core._hub_crud import select_instance_by_id\n",
"from lamindb_setup.core._hub_core import get_storage_records_for_instance\n",
"assert call_with_fallback_auth(select_instance_by_id, instance_id=instance_id.hex) is None\n",
"\n",
"assert (\n",
" call_with_fallback_auth(select_instance_by_id, instance_id=instance_id.hex) is None\n",
")\n",
"assert not get_storage_records_for_instance(instance_id)"
]
}
Expand Down
10 changes: 8 additions & 2 deletions docs/hub-cloud/05-init-hosted-instance.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
"metadata": {},
"outputs": [],
"source": [
"assert ln_setup.settings.instance.storage.type_is_cloud == True\n",
"assert ln_setup.settings.instance.storage.type_is_cloud is True\n",
"assert ln_setup.settings.instance.owner == ln_setup.settings.user.handle\n",
"assert ln_setup.settings.instance.name == \"my-hosted\"\n",
"assert ln_setup.settings.storage.root.as_posix().startswith(HOSTED_BUCKETS)\n",
Expand All @@ -114,7 +114,13 @@
"outputs": [],
"source": [
"hub = connect_hub_with_auth()\n",
"response = hub.table(\"storage\").select(\"*\").eq(\"root\", ln_setup.settings.storage.root.as_posix()).execute().data\n",
"response = (\n",
" hub.table(\"storage\")\n",
" .select(\"*\")\n",
" .eq(\"root\", ln_setup.settings.storage.root.as_posix())\n",
" .execute()\n",
" .data\n",
")\n",
"assert len(response) == 1\n",
"assert response[0][\"is_default\"]"
]
Expand Down
2 changes: 1 addition & 1 deletion docs/hub-cloud/06-connect-hosted-instance.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
"outputs": [],
"source": [
"target_dir = root / \"test-dir-upload\"\n",
"target_dir.upload_from(test_dir, create_folder=True) # default\n",
"target_dir.upload_from(test_dir, create_folder=True) # default\n",
"\n",
"assert target_dir.is_dir()\n",
"assert (target_dir / \"test-dir-upload\").exists()\n",
Expand Down
22 changes: 13 additions & 9 deletions docs/hub-cloud/07-keep-artifacts-local.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,10 @@
"\n",
"assert ln_setup.settings.instance.name == name\n",
"assert ln_setup.settings.instance.storage.type_is_cloud\n",
"assert (\n",
" ln_setup.settings.instance.storage.root_as_str\n",
" == storage\n",
")\n",
"assert ln_setup.settings.instance.storage.root_as_str == storage\n",
"assert (\n",
" ln_setup.settings.instance._sqlite_file.as_posix()\n",
" == f\"{storage}/{ln_setup.settings.instance._id.hex}.lndb\" # noqa\n",
" == f\"{storage}/{ln_setup.settings.instance._id.hex}.lndb\"\n",
")"
]
},
Expand All @@ -47,7 +44,10 @@
"source": [
"with pytest.raises(ValueError) as error:\n",
" ln_setup.settings.instance.storage_local\n",
"assert error.exconly() == \"ValueError: `keep_artifacts_local` is not enabled for this instance.\""
"assert (\n",
" error.exconly()\n",
" == \"ValueError: `keep_artifacts_local` is not enabled for this instance.\"\n",
")"
]
},
{
Expand All @@ -74,7 +74,9 @@
" ln_setup.settings.instance.storage_local.root.as_posix()\n",
" == UPath(\"./my_storage_local\").resolve().as_posix()\n",
")\n",
"assert (ln_setup.settings.instance.storage_local.root / \".lamindb/_is_initialized\").read_text() == ln_setup.settings.instance.storage_local.uid\n",
"assert (\n",
" ln_setup.settings.instance.storage_local.root / \".lamindb/_is_initialized\"\n",
").read_text() == ln_setup.settings.instance.storage_local.uid\n",
"assert ln_setup.settings.instance.storage_local is not None\n",
"# the remote storage location is still in the regular slot\n",
"assert ln_setup.settings.instance.storage.root.as_posix() == storage"
Expand Down Expand Up @@ -106,7 +108,9 @@
" ln_setup.settings.instance.storage_local.root.as_posix()\n",
" == UPath(\"./my_storage_local2\").resolve().as_posix()\n",
")\n",
"assert (ln_setup.settings.instance.storage_local.root / \".lamindb/_is_initialized\").read_text() == ln_setup.settings.instance.storage_local.uid"
"assert (\n",
" ln_setup.settings.instance.storage_local.root / \".lamindb/_is_initialized\"\n",
").read_text() == ln_setup.settings.instance.storage_local.uid"
]
},
{
Expand Down Expand Up @@ -154,7 +158,7 @@
"metadata": {},
"outputs": [],
"source": [
"test_file = (ln_setup.settings.instance.storage_local.root / \".lamindb/test_file.txt\")\n",
"test_file = ln_setup.settings.instance.storage_local.root / \".lamindb/test_file.txt\"\n",
"test_file.write_text(\"test\")"
]
},
Expand Down
59 changes: 45 additions & 14 deletions docs/hub-prod/test-cloud-sync.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
"source": [
"import os\n",
"\n",
"instance_name = f\"test-sqlite-sync\"\n",
"instance_name = \"test-sqlite-sync\"\n",
"!lamin connect {instance_name}\n",
"!yes | lamin delete {instance_name}"
]
Expand All @@ -33,7 +33,6 @@
"from lamindb_setup.core.upath import UPath, LocalPathClasses\n",
"import shutil\n",
"import time\n",
"import os\n",
"import pytest"
]
},
Expand Down Expand Up @@ -131,8 +130,14 @@
"source": [
"test_local_path = UPath(\"./some/local/path\")\n",
"assert settings.paths.cloud_to_local_no_update(test_local_path) == test_local_path\n",
"assert settings.paths.cloud_to_local_no_update(test_local_path.as_posix()) == test_local_path\n",
"assert settings.paths.cloud_to_local_no_update(test_local_path, cache_key=\"some/cache/key\") == test_local_path"
"assert (\n",
" settings.paths.cloud_to_local_no_update(test_local_path.as_posix())\n",
" == test_local_path\n",
")\n",
"assert (\n",
" settings.paths.cloud_to_local_no_update(test_local_path, cache_key=\"some/cache/key\")\n",
" == test_local_path\n",
")"
]
},
{
Expand All @@ -142,11 +147,25 @@
"metadata": {},
"outputs": [],
"source": [
"assert settings.paths.cloud_to_local_no_update(dir_sync) == settings.cache_dir / f\"lamindb-ci/{instance_name}/dir_sync\"\n",
"assert settings.paths.cloud_to_local_no_update(dir_sync.as_posix()) == settings.cache_dir / f\"lamindb-ci/{instance_name}/dir_sync\"\n",
"assert (\n",
" settings.paths.cloud_to_local_no_update(dir_sync)\n",
" == settings.cache_dir / f\"lamindb-ci/{instance_name}/dir_sync\"\n",
")\n",
"assert (\n",
" settings.paths.cloud_to_local_no_update(dir_sync.as_posix())\n",
" == settings.cache_dir / f\"lamindb-ci/{instance_name}/dir_sync\"\n",
")\n",
"\n",
"assert settings.paths.cloud_to_local_no_update(dir_sync, cache_key=\"dir_cache/key\") == settings.cache_dir / \"dir_cache/key\"\n",
"assert settings.paths.cloud_to_local_no_update(dir_sync.as_posix(), cache_key=\"dir_cache/key\") == settings.cache_dir / \"dir_cache/key\""
"assert (\n",
" settings.paths.cloud_to_local_no_update(dir_sync, cache_key=\"dir_cache/key\")\n",
" == settings.cache_dir / \"dir_cache/key\"\n",
")\n",
"assert (\n",
" settings.paths.cloud_to_local_no_update(\n",
" dir_sync.as_posix(), cache_key=\"dir_cache/key\"\n",
" )\n",
" == settings.cache_dir / \"dir_cache/key\"\n",
")"
]
},
{
Expand All @@ -157,7 +176,9 @@
"outputs": [],
"source": [
"# for http urls\n",
"http_path = UPath(\"https://raw.githubusercontent.com/laminlabs/lamindb-setup/refs/heads/main/README.md\")\n",
"http_path = UPath(\n",
" \"https://raw.githubusercontent.com/laminlabs/lamindb-setup/refs/heads/main/README.md\"\n",
")\n",
"assert http_path.protocol == \"https\"\n",
"\n",
"http_stat = http_path.stat()\n",
Expand All @@ -175,9 +196,17 @@
"source": [
"http_key = \"raw.githubusercontent.com/laminlabs/lamindb-setup/refs/heads/main/README.md\"\n",
"\n",
"assert settings.paths.cloud_to_local_no_update(http_path) == settings.cache_dir / http_key\n",
"assert settings.paths.cloud_to_local_no_update(str(http_path)) == settings.cache_dir / http_key\n",
"assert settings.paths.cloud_to_local_no_update(http_path, cache_key=\"check/README.md\") == settings.cache_dir / \"check/README.md\""
"assert (\n",
" settings.paths.cloud_to_local_no_update(http_path) == settings.cache_dir / http_key\n",
")\n",
"assert (\n",
" settings.paths.cloud_to_local_no_update(str(http_path))\n",
" == settings.cache_dir / http_key\n",
")\n",
"assert (\n",
" settings.paths.cloud_to_local_no_update(http_path, cache_key=\"check/README.md\")\n",
" == settings.cache_dir / \"check/README.md\"\n",
")"
]
},
{
Expand All @@ -195,7 +224,9 @@
"metadata": {},
"outputs": [],
"source": [
"dir_sync_local = settings.paths.cloud_to_local(dir_sync.as_posix(), cache_key=\"dir_cache/key\")"
"dir_sync_local = settings.paths.cloud_to_local(\n",
" dir_sync.as_posix(), cache_key=\"dir_cache/key\"\n",
")"
]
},
{
Expand Down Expand Up @@ -410,7 +441,7 @@
"time.sleep(1)\n",
"cloud_file = dir_sync / \"file1\"\n",
"# update cloud timestamp\n",
"cloud_file.fs.touch(cloud_file.as_posix(), truncate=True) \n",
"cloud_file.fs.touch(cloud_file.as_posix(), truncate=True)\n",
"\n",
"assert cloud_file.modified.timestamp() > local_file_new.stat().st_mtime"
]
Expand Down
5 changes: 4 additions & 1 deletion docs/hub-prod/test-sqlite-lock.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,10 @@
"ln_setup.init(storage=\"s3://lamindb-ci/test-load-lock\", name=\"test-load-lock\")\n",
"instance_id = ln_setup.settings.instance._id\n",
"\n",
"assert ln_setup.settings.instance._cloud_sqlite_locker is ln_setup.settings.instance._cloud_sqlite_locker\n",
"assert (\n",
" ln_setup.settings.instance._cloud_sqlite_locker\n",
" is ln_setup.settings.instance._cloud_sqlite_locker\n",
")\n",
"\n",
"ln_setup.close()"
]
Expand Down
2 changes: 1 addition & 1 deletion lamindb_setup/core/upath.py
Original file line number Diff line number Diff line change
Expand Up @@ -865,7 +865,7 @@ def check_storage_is_empty(
)
if n_diff > 0:
if raise_error:
raise InstanceNotEmpty(message)
raise InstanceNotEmpty(message) from None
else:
logger.warning(message)
return n_diff
Loading

0 comments on commit 58f4749

Please sign in to comment.