-
Notifications
You must be signed in to change notification settings - Fork 332
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
Feature/revive integration tests (#1343) #1346
Conversation
* add postgres to integration * add postgres to integration * up * rename * hardcode * add back postgres * add back postgres * add pgvector * add pgvector * add pgvector * add pgvector * add pgvector * add pgvector * add pgvector * tweak config docs * fix integration suite * fix integration suite * fix integration suite * up * up * up * up * up * up * up * up * up * update integration test * final user tests * final user tests
…sary error on deletion (#1344) * Don't throw an error when deleting a collection with no documents, fix return on create collection, more js tests * Revert "Don't throw an error when deleting a collection with no documents, fix return on create collection, more js tests" This reverts commit f9f6ead. * Don't throw an error when deleting a collection with no documents, fix return on create collection, more js tests * more
* try * up * up * space * add it in ingestion * rm ingestion * init * add semaphore * test * rm duplicates * kg_creation_settings * rm semaphores * increase conns * change it back * clean * up * up * up * up --------- Co-authored-by: --global=Shreyas Pimpalgaonkar <--global=shreyas.gp.7@gmail.com> Co-authored-by: emrgnt-cmplxty <68796651+emrgnt-cmplxty@users.noreply.github.com>
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.
👍 Looks good to me! Reviewed everything up to 2fb8fd7 in 46 seconds
More details
- Looked at
2201
lines of code in38
files - Skipped
1
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. py/tests/integration/harness_cli.py:145
- Draft comment:
The print statement should indicate that this is a hybrid search test, not a vector search test. - Reason this comment was not posted:
Confidence changes required:50%
The test functiontest_hybrid_search_sample_file_filter_cli
is incorrectly labeled as testing vector search instead of hybrid search. This could lead to confusion when reading the test logs or debugging.
2. py/tests/integration/harness_cli.py:186
- Draft comment:
Consider implementing JSON parsing for the RAG response to improve test robustness and maintainability. - Reason this comment was not posted:
Confidence changes required:50%
The test functiontest_rag_response_sample_file_cli
has a TODO comment indicating a potential improvement to parse JSON output. This should be addressed to ensure the test is robust and maintainable.
3. py/tests/integration/harness_sdk.py:272
- Draft comment:
Avoid reassigning theresponse
variable. Use a different variable name for accumulating the response content. - Reason this comment was not posted:
Confidence changes required:50%
The test functiontest_rag_response_stream_sample_file_sdk
reassigns theresponse
variable, which can lead to confusion and potential bugs. It's better to use a different variable name for the accumulated response.
4. py/tests/integration/harness_sdk.py:348
- Draft comment:
Useoutput
instead ofresponse_content
in the final check to match the accumulated content variable. - Reason this comment was not posted:
Confidence changes required:50%
In the functiontest_agent_stream_sample_file_sdk
, the variableresponse
is used to accumulate content, but the final check usesresponse_content
, which is undefined. This is likely a typo and should be corrected.
Workflow ID: wflow_f9rKHXcq1zeYIH81
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
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.
👍 Looks good to me! Incremental review on 2df2e8a in 26 seconds
More details
- Looked at
13
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_JWrEU9HZSf4pBimc
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
* Feature/revive integration tests (SciPhi-AI#1343) * add postgres to integration * add postgres to integration * up * rename * hardcode * add back postgres * add back postgres * add pgvector * add pgvector * add pgvector * add pgvector * add pgvector * add pgvector * add pgvector * tweak config docs * fix integration suite * fix integration suite * fix integration suite * up * up * up * up * up * up * up * up * up * update integration test * final user tests * final user tests * Fix validation error on collection creation responses, remove unnecessary error on deletion (SciPhi-AI#1344) * Don't throw an error when deleting a collection with no documents, fix return on create collection, more js tests * Revert "Don't throw an error when deleting a collection with no documents, fix return on create collection, more js tests" This reverts commit f9f6ead. * Don't throw an error when deleting a collection with no documents, fix return on create collection, more js tests * more * Improve kg throughput (SciPhi-AI#1342) * try * up * up * space * add it in ingestion * rm ingestion * init * add semaphore * test * rm duplicates * kg_creation_settings * rm semaphores * increase conns * change it back * clean * up * up * up * up --------- Co-authored-by: --global=Shreyas Pimpalgaonkar <--global=shreyas.gp.7@gmail.com> Co-authored-by: emrgnt-cmplxty <68796651+emrgnt-cmplxty@users.noreply.github.com> --------- Co-authored-by: Nolan Tremelling <34580718+NolanTrem@users.noreply.github.com> Co-authored-by: Shreyas Pimpalgaonkar <shreyas.gp.7@gmail.com> Co-authored-by: --global=Shreyas Pimpalgaonkar <--global=shreyas.gp.7@gmail.com>
add postgres to integration
add postgres to integration
up
rename
hardcode
add back postgres
add back postgres
add pgvector
add pgvector
add pgvector
add pgvector
add pgvector
add pgvector
add pgvector
tweak config docs
fix integration suite
fix integration suite
fix integration suite
up
up
up
up
up
up
up
up
up
update integration test
final user tests
final user tests
Important
Revive integration tests by adding PostgreSQL and pgvector support, updating configurations, and enhancing test coverage.
pgvector
tointegration-test-workflow-debian.yml
.harness_cli.py
andharness_sdk.py
to cover ingestion, search, and user management.r2r.toml
and other config files to reflect new chunking strategy and PostgreSQL settings.concurrent_request_limit
to 256 inr2r.toml
andr2r_aws_bedrock.toml
.create_ingestion_provider()
infactory.py
to handleIngestionConfig
as a dictionary.get_user_app()
inauth_router.py
to useUUID
foruser_id
.PostgresRelationalDBProvider
andPostgresVectorDBProvider
to use semaphores for connection management.r2rClientIntegrationSuperUser.test.ts
for collection management.This description was created by for 2df2e8a. It will automatically update as commits are pushed.