-
Notifications
You must be signed in to change notification settings - Fork 0
feat: default database and collection in MetadataDbClient #152
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
Conversation
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.
General comment: should we set this up so that it pulls from environment variables but has defaults in place? Like in the QC and metadata portals right now I use
API_GATEWAY_HOST = os.getenv("API_GATEWAY_HOST", "api.allenneuraldynamics-test.org")
DATABASE = os.getenv("DATABASE", "metadata_index")
COLLECTION = os.getenv("COLLECTION", "data_assets")
If we set up some generic env variable names here like "DATA_ACCESS_API_HOST", "_DB", and "_COLLECTION" here then in deployments we would have a no-code way of switching apps between dev/prod just from the aws param store. Same with version, could have a default in place, pull from an env variable when present, and still be user-settable for people who need to set it in their code directly.
We could potentially set something up where it searches for configuration settings in an env file or env vars, but I reckon we can do that in a future PR as a nice-to-have. |
Yeah I agree it could be a useful addition. In the SSH client, we did something like that using |
docs/source/ExamplesDocDBRestApi.rst
Outdated
|
||
docdb_api_client = MetadataDbClient( | ||
host=API_GATEWAY_HOST, | ||
database=DATABASE, | ||
collection=COLLECTION, | ||
# database=DATABASE, |
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.
It might be better to remove these comments and just make a note above on how to override the fields if needed.
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.
Updated
closes #151
Default version is v1, which uses metadata_index/data_assets:
v2 uses new metadata_index_v2/data_assets.