-
Notifications
You must be signed in to change notification settings - Fork 2
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
Streamline optional dependency sqlmodel #116
Comments
One way I'm doing this in geniml that is working is something like this: # first, set a const so we know whether the dep is satisfied or not
try:
import hnswlib
DEP_HNSWLIB = True
except ImportError:
DEP_HNSWLIB = False
_LOGGER.error(
"HNSWBackend requires hnswlib. Install hnswlib, or ignore this if you don't need HNSWBackend"
)
if not DEP_HNSWLIB:
# Dependency not met; make dummy class so code can instantiate, but it brakes if actually used
class HNSWBackend(EmSearchBackend):
pass
else:
# dependency met, make the real class
class HNSWBackend(EmSearchBackend):
"""A search backend that uses a local HNSW index to store and search embeddings"""
... it's nice to keep this logic to a single place to make maintenance easier |
This is currently the code for checking dependencies for the DB backend (within pipestat.py).: Lines 55 to 90 in 37c7311
To answer the above question
Lines 55-90 will allow for sqlmodel to not be installed. Also, I do think that the lines from dbhelpers.py do not need to be in a try: except: pipestat/pipestat/backends/db_backend/db_helpers.py Lines 5 to 8 in 37c7311
|
In
db_helpers.py
, we're using a try block to wrap the sqlmodel import:But in
db_parsed_schema.py
, anddbbackend.py
we are not:Will this break things if sqlmodel is not installed?
Also, is there a better way to isolate the sqlmodel optionality into a single place? For example, I'm thinking of something like importing sqlmodel in an
__init__.py under
backends/db_backend` and then having these show up in everything in that module, so that it doesn't have to be repeatedly imported? (not sure if this will work)The text was updated successfully, but these errors were encountered: