-
Notifications
You must be signed in to change notification settings - Fork 15
🚸 Always import the entire public API & enable switching instances in the same Python session #2851
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2851 +/- ##
==========================================
- Coverage 91.84% 91.58% -0.27%
==========================================
Files 69 67 -2
Lines 10807 9448 -1359
==========================================
- Hits 9926 8653 -1273
+ Misses 881 795 -86 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Ok, now tests are working. |
Deployment URL: https://1623a0f9.lamindb.pages.dev |
tests/storage/conftest.py
Outdated
@@ -51,7 +51,7 @@ def pytest_sessionstart(): | |||
quit() | |||
total_time_elapsed = perf_counter() - t_execute_start | |||
print(f"time to setup the instance: {total_time_elapsed:.1f}s") | |||
assert ln.Storage.filter(root="s3://lamindb-ci/test-data").one_or_none() is not None | |||
# assert ln.Storage.filter(root="s3://lamindb-ci/test-data").one_or_none() is not None |
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.
Why is it commented out?
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.
There indeed was an issue here that I hadn't yet caught. Now dealt with here: laminlabs/lamindb-setup@7523b97
This is now maybe the remaining issue: some modules that might import lamindb somewhere. But we'll gather some experience for this and I think it's less bad than the constant errors you obtain when you try to normally develop with lamindb without all these changes of this PR.
Behavior if no instance is configured:
No solution yet for when the user attempts queries -- challenge is that we need to pass a valid db connection string to Django and then Django would need to be intercepted later
>>> ln.Artifact.df() Traceback (most recent call last): File "/Users/falexwolf/miniconda3/envs/py312/lib/python3.12/site-packages/django/db/backends/utils.py", line 105, in _execute return self.cursor.execute(sql, params) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Users/falexwolf/miniconda3/envs/py312/lib/python3.12/site-packages/django/db/backends/sqlite3/base.py", line 354, in execute return super().execute(query, params) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ sqlite3.OperationalError: no such table: lamindb_artifact
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "", line 1, in
File "/Users/falexwolf/repos/laminhub/rest-hub/sub/lamindb/lamindb/models/sqlrecord.py", line 505, in df
return query_set[:limit].df(include=include, features=features)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/falexwolf/repos/laminhub/rest-hub/sub/lamindb/lamindb/models/query_set.py", line 650, in df
df = pd.DataFrame(queryset.values(*field_names, *list(annotate_kwargs.keys())))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/falexwolf/miniconda3/envs/py312/lib/python3.12/site-packages/pandas/core/frame.py", line 843, in init
data = list(data)
^^^^^^^^^^
File "/Users/falexwolf/miniconda3/envs/py312/lib/python3.12/site-packages/django/db/models/query.py", line 381, in iter
self._fetch_all()
File "/Users/falexwolf/miniconda3/envs/py312/lib/python3.12/site-packages/django/db/models/query.py", line 1909, in _fetch_all
self._result_cache = list(self._iterable_class(self))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/falexwolf/miniconda3/envs/py312/lib/python3.12/site-packages/django/db/models/query.py", line 213, in iter
for row in compiler.results_iter(
^^^^^^^^^^^^^^^^^^^^^^
File "/Users/falexwolf/miniconda3/envs/py312/lib/python3.12/site-packages/django/db/models/sql/compiler.py", line 1531, in results_iter
results = self.execute_sql(
^^^^^^^^^^^^^^^^^
File "/Users/falexwolf/miniconda3/envs/py312/lib/python3.12/site-packages/django/db/models/sql/compiler.py", line 1580, in execute_sql
cursor.execute(sql, params)
File "/Users/falexwolf/miniconda3/envs/py312/lib/python3.12/site-packages/django/db/backends/utils.py", line 79, in execute
return self._execute_with_wrappers(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/falexwolf/miniconda3/envs/py312/lib/python3.12/site-packages/django/db/backends/utils.py", line 92, in _execute_with_wrappers
return executor(sql, params, many, context)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/falexwolf/miniconda3/envs/py312/lib/python3.12/site-packages/django/db/backends/utils.py", line 100, in _execute
with self.db.wrap_database_errors:
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/falexwolf/miniconda3/envs/py312/lib/python3.12/site-packages/django/db/utils.py", line 91, in exit
raise dj_exc_value.with_traceback(traceback) from exc_value
File "/Users/falexwolf/miniconda3/envs/py312/lib/python3.12/site-packages/django/db/backends/utils.py", line 105, in _execute
return self.cursor.execute(sql, params)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/falexwolf/miniconda3/envs/py312/lib/python3.12/site-packages/django/db/backends/sqlite3/base.py", line 354, in execute
return super().execute(query, params)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
django.db.utils.OperationalError: no such table: lamindb_artifact
Behavior upon trying to toggle auto-connect:
Credit goes to @Koncopd through the below change that enables switching instances in the same python session:
The other important change that enabled removing the gated models import is here:
This enables a few simplifications in the CLI:
auto-connect
switches lamin-cli#138Supersedes:
lamindb.models
#2849