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

Changed document_store to ElasticsearchDocumentStore #2192

Merged
merged 1 commit into from Feb 16, 2022
Merged

Changed document_store to ElasticsearchDocumentStore #2192

merged 1 commit into from Feb 16, 2022

Conversation

mkkuemmel
Copy link
Member

because FAISSDocumentStore errored with ElasticSearchRetriever

Proposed changes:

  • change DocumentStore as the FAISSDocumentstore is not compatible with the ElasticSearchRetriever used later on
  • in the Notebook version of this tutorial there is no FAISSDocumentstore being used either

Status (please check what you already did):

  • First draft (up for discussions & feedback)
  • Final code
  • Added tests
  • Updated documentation

@mkkuemmel
changed document_store to ElasticsearchDocumentStore
c2676c4 
because FAISSDocumentStore errored with ElasticSearchRetriever
@ZanSara
Copy link
Contributor

ZanSara commented Feb 16, 2022 •
edited

Wow, thanks! I'm amazed nobody noticed this for so long! 😄 I'll go ahead and approve this if you don't mind, so you can merge 🙂

(This was accidentally introduced by me in one large PR where the reviewers had way too much stuff to check, so definitely my bad)

ZanSara
ZanSara approved these changes Feb 16, 2022
View reviewed changes
Copy link
Contributor

@ZanSara ZanSara left a comment

Choose a reason for hiding this comment

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

👍

Timoeller
Timoeller approved these changes Feb 16, 2022
View reviewed changes
Copy link
Contributor

@Timoeller Timoeller left a comment

Choose a reason for hiding this comment

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

: )

@mkkuemmel mkkuemmel merged commit 853de93 into master Feb 16, 2022
@mkkuemmel mkkuemmel deleted the fix_tutorial1_faissdocumentstore branch February 16, 2022 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Timoeller Timoeller Timoeller approved these changes

@ZanSara ZanSara ZanSara approved these changes

Assignees
No one assigned
Labels
topic:document_store type:bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mkkuemmel @ZanSara @Timoeller