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.

Sign up for GitHub

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

Jump to bottom

Kendra retriever api #6616

Merged
merged 9 commits into from
Jun 23, 2023
Merged

Kendra retriever api #6616

merged 9 commits into from
Jun 23, 2023

Conversation

3coins
Copy link
Contributor

@3coins 3coins commented Jun 22, 2023

Description

Replaces Kendra Retriever with an updated version that uses the new retriever API which is better suited for retrieval augmented generation (RAG) systems.

Note: This change requires the latest version (1.26.159) of boto3 to work. pip install -U boto3 to upgrade the boto3 version.

cc @hupe1980
cc @dev2049

@vercel
Copy link

vercel bot commented Jun 22, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Jun 23, 2023 8:30pm

@3coins 3coins force-pushed the kendra-retriever-api branch 7 times, most recently from 7a38c68 to aaa228d Compare June 23, 2023 01:38
@3coins 3coins force-pushed the kendra-retriever-api branch from aaa228d to ca7cfe0 Compare June 23, 2023 01:42
@3coins 3coins marked this pull request as ready for review June 23, 2023 01:48
@hupe1980
Copy link
Contributor

hupe1980 commented Jun 23, 2023 •
edited
Loading

@3coins I think there is a small error in it: getRelevantsDocuments signature is incorrect. You have to set the params top_k and attribute_filter ​​in the constructor:

https://github.com/3coins/langchain/blob/ca7cfe0591bef677e10f97b80cde939e4043c115/langchain/schema.py#L310C1-L314C1

@hupe1980 hupe1980 mentioned this pull request Jun 23, 2023
Merged
@3coins
Copy link
Contributor Author

3coins commented Jun 23, 2023

@hupe1980
Thanks for calling out the change. I will update shortly and resubmit.

@3coins 3coins marked this pull request as draft June 23, 2023 14:14
@3coins 3coins marked this pull request as ready for review June 23, 2023 14:52
@3coins
Copy link
Contributor Author

3coins commented Jun 23, 2023

@hupe1980 @dev2049
Submitted updates as per your comment. This should be good to merge now.

leo-gan
leo-gan approved these changes Jun 23, 2023
View reviewed changes
Copy link
Collaborator

@leo-gan leo-gan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM Yet, integration tests could be helpful.

@3coins
Copy link
Contributor Author

3coins commented Jun 23, 2023

@leo-gan
Thanks for approving. I will be adding tests in the next iteration. There is a Kendra samples repo that is dependent on this change, so this should be merged and released.

@dev2049 dev2049 self-requested a review June 23, 2023 19:29
@3coins
Copy link
Contributor Author

3coins commented Jun 23, 2023

@dev2049
Thanks for reviewing. Took care of the class attributes. Let me know if there is anything else I should update.

@3coins 3coins requested a review from dev2049 June 23, 2023 21:02
@dev2049
Copy link
Contributor

dev2049 commented Jun 23, 2023

lgtm, thanks @3coins !

@dev2049 dev2049 merged commit b1de927 into langchain-ai:master Jun 23, 2023
dev2049
dev2049 reviewed Jun 23, 2023
View reviewed changes
docs/extras/modules/data_connection/retrievers/integrations/amazon_kendra_retriever.ipynb
Copy link
Contributor

Choose a reason for hiding this comment

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

just noticed there's an index_id left in the notebook — is that intentional?

@ellisonbg
Copy link

Thanks everyone!

This was referenced Jun 25, 2023
Closed
Merged
@3coins 3coins mentioned this pull request Jun 27, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dev2049 dev2049 dev2049 left review comments

@leo-gan leo-gan leo-gan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@3coins @hupe1980 @dev2049 @ellisonbg @leo-gan