Skip to content
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

add get uniq #191

Merged
merged 1 commit into from
Mar 21, 2024
Merged

add get uniq #191

merged 1 commit into from
Mar 21, 2024

Conversation

emrgnt-cmplxty
Copy link
Contributor

@emrgnt-cmplxty emrgnt-cmplxty commented Mar 21, 2024

Ellipsis 🚀 This PR description was created by Ellipsis for commit 0b281f9.

Summary:

This PR introduces a new method for retrieving unique values from the database and adds two new endpoints for fetching user IDs and user documents.

Key points:

  • Added get_all_unique_values method in VectorDBProvider and its implementations in LocalVectorDB, PGVectorDB, and QdrantDB.
  • Added new endpoints /get_user_ids/ and /get_user_documents/ in FastAPI application.
  • No changes in run_client.py.

Generated with ❤️ by ellipsis.dev

@emrgnt-cmplxty emrgnt-cmplxty merged commit 0529830 into main Mar 21, 2024
2 checks passed
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested.

  • Reviewed the entire pull request up to 0b281f9
  • Looked at 217 lines of code in 6 files
  • Took 1 minute and 17 seconds to review
More info
  • Skipped 0 files when reviewing.
  • Skipped posting 1 additional comments because they didn't meet confidence threshold of 50%.
1. r2r/main/app.py:296:
  • Assessed confidence : 50%
  • Comment:
    Consider adding type hints to the return values of the new endpoints for better clarity and consistency with the rest of the codebase. For example, for the get_user_ids endpoint, you could add -> Dict[str, List[str]]: to the function signature.
  • Reasoning:
    The PR adds a new method get_all_unique_values to the VectorDBProvider abstract base class and its implementations. This method retrieves all unique values for a given metadata field, optionally filtered by other metadata fields. The method is correctly implemented in the LocalVectorDB, PGVectorDB, and QdrantDB classes. The PR also adds two new endpoints to the FastAPI app: /get_user_ids/ and /get_user_documents/, which use the new method to retrieve unique user IDs and document IDs, respectively. The code looks correct, but there are a few minor improvements that could be made.

Workflow ID: wflow_ikZxasOsR7qtGlMv


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. We'll respond in a few minutes. Learn more here.

):
print("rag_completion")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the print statement. It seems to be a debugging statement and should not be in the production code.

Suggested change
print("rag_completion")

@emrgnt-cmplxty emrgnt-cmplxty deleted the feature/add-get-uniq branch March 21, 2024 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant