Using a DbSession and an EngineSingleton to inject the engine #199
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…d of giving it to every router. This is cleaner (I think) and makes it easy to patch the engine for unittests. Note that FastAPI Depends would work as well for the routers, but not for other parts of the code.
josvandervelde
added a commit
that referenced
this pull request
Nov 20, 2023
…eated when ES_USER and ES_PASSWORD env vars are empty; used the style of PR #199
Taniya-Das
reviewed
Nov 21, 2023
Taniya-Das
reviewed
Nov 21, 2023
src/tests/routers/ai_asset_routers/test_router_aiassets_retrieve_content.py
Show resolved
Hide resolved
Taniya-Das
reviewed
Nov 21, 2023
Taniya-Das
reviewed
Nov 21, 2023
Taniya-Das
requested changes
Nov 21, 2023
Thanks for the review! I indeed forgot to remove many usages of the |
josvandervelde
deleted the
feature/cleaner-sql-engine-dependency-injection
branch
AdrianAlcolea
added a commit
that referenced
this pull request
Nov 28, 2023
* First approach to use the elasticsearch service * elasticsearch query example * elasticsearch query complete example * Elasticsearch setup configured * Remove .DS_Store and add it to dockerignore * Created an elastic search endpoint. It isnt working yet * Just to test the new database * To rebase develop * Working but not up-to-date with develop branch * Service working. Need to get up-to-date with develop * Some bugfixes for the connectors * Deleting docker images in clean script * Fixed issues with authentication * Publication search seems to work. TODO: test cases and other resourcse * platform and platform_identifier changed from aiod_entry to each instance * Created testcase for publication search * Made ElasticSearch router generic, implemented it for dataset * Logstash configured for dataset, experiment, ml_model, publication, and service * Logstash configured for dataset, experiment, ml_model, publication, and service * Logstash waits until fill-db-with-examples ends * take src from develop * Copied entire develop branch * Logstash configuration readapted to new names * Logstash configuration readapted to new names * added ai4experiments to platform names * Copied initial search routers to start creating them * Examples of ml_model, dataset and experiment used to insert ai4experiment data * Descriptions of the ai4experiment data improved * platform added to mappings * elasticsearch query example completed * First version of search service working * Search router tests implemented * Search fields selection added * Added search for event, news, ortganisation and project * Added routers for event, news, organisation and project * Logstash names changed * added logstash_config.py, just for having it there * Pagination changed to actual pages * Pagination changed to actual pages * Application areas added to elasticsearch resuts * First version with deletion * Prepared to be merged with develop * pull request modifications * pull request modifications * pull request modifications * Combined search with sql queries in process * Search functionality combined with optional SQL statment to retrieve everything * Elasticsearch and logstash configuration integrated in src * Search router tests actualised * Search router tests actualised * Search router tests actualised * pre-commit passed * All test passed and working. Not merged with develop * huggingface connector test to its original state * back to commented huggingface connector * Fixing unittests by making sure Elasticsearch instance can also be created when ES_USER and ES_PASSWORD env vars are empty; used the style of PR #199 * clean logstash configuration * clean logstash configuration * clean logstash configuration * clean logstash configuration * clean logstash configuration * clean logstash configuration * logstash config files generated with jinja2 * logstash config files generated with jinja2 * Logstash config files generated with jinja2. All test passed, but not merged with develop. * Second round of pull request comments * Second round of pull request comments * Second round of pull request comments * Second round of pull request comments * Second round of pull request comments * Created data/elasticsearch/.gitkeep to make sure it exists with the right permissions * Deleted autogenerated file logstash/config/logstash.yml * cleanup * Making sure docker compose up works even if generated files do not exist; added logging; simplified file names * Make sure data folders are always created with correct permissions (this was by accident removed in commit cc8c22f) * Added default logstash configuration * Fixed docker compose * Using FastAPI input validation * Made status nullable, so that we can return an empty status in the search_router --------- Co-authored-by: Adrián <[email protected]> Co-authored-by: Jos van der Velde <[email protected]>
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Small refactor to make the code a bit cleaner.
Using a DbSession and an EngineSingleton to inject the engine, instead of giving it to every router. This is cleaner (I think) and makes it easy to patch the engine for unittests. Note that FastAPI Depends would work as well for the routers, but not for other parts of the code.