-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Added search quality testing pipeline #1774
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Generally looks good, didn't scrutinize the scripts that much but seems reasonable.
By the way, there are a ton of errors in the automated checks. I know this is more of a [WIP] PR but let's definitely fix those up before the final PR
backend/tests/regression/answer_quality/search_test_config.yaml
Outdated
Show resolved
Hide resolved
backend/tests/regression/answer_quality/search_test_config.yaml
Outdated
Show resolved
Hide resolved
backend/tests/regression/answer_quality/search_test_config.yaml
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,406 @@ | |||
version: '3' |
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.
I'm quite against having yet another docker compose yaml. Every time we add a new env based config we have to currently copy it to 3 docker compose yamls, kubernetes deployment, helm deployment. Let's see if we can just use the existing yamls
ports: | ||
- "8080" | ||
environment: | ||
# Auth Settings |
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.
this can rely on an .env file explicitly. The goal is to not have to duplicate the env vars so that when we introduce a new one it has to be changed in 20 places
o: bind | ||
device: ${DANSWER_VESPA_DATA_DIR:-./vespa_data} | ||
model_cache_huggingface: | ||
# driver: local |
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 cleanup here (also other commented things around the PR). Be sure to give the PR a read on GitHub before the final review <3 thanks!
# driver_opts: | ||
# type: none | ||
# o: bind | ||
# device: ${DANSWER_MODEL_CACHE_DIR:-./model_cache} |
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.
NEWLINE REEEE
- LANGUAGE_CHAT_NAMING_HINT=${LANGUAGE_CHAT_NAMING_HINT:-} | ||
- QA_PROMPT_OVERRIDE=${QA_PROMPT_OVERRIDE:-} | ||
# Other Services | ||
- POSTGRES_HOST=relational_db |
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.
Some of the env things do still need to stay as it points to a service in the docker compose deployment which is not default
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.
Super amazing work! Thanks!
No description provided.