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

Add elasticsearch-haystack package #41

Merged
merged 21 commits into from
Nov 10, 2023
Merged

Add elasticsearch-haystack package #41

merged 21 commits into from
Nov 10, 2023

Conversation

silvanocerza
Copy link
Contributor

This is the identical code from deepset-ai/elasticsearch-haystack.

README.md and pyproject.toml are the only different files.

It also adds a workflow to run its tests.

@silvanocerza silvanocerza requested a review from a team as a code owner October 16, 2023 13:26
@silvanocerza silvanocerza requested review from ZanSara and removed request for a team October 16, 2023 13:26
@silvanocerza silvanocerza self-assigned this Oct 16, 2023
@silvanocerza
Copy link
Contributor Author

Putting this on hold for the time being, I'll focus on reworking the Document dataclass in Haystack hoping it will make easier to fix the filtering. Then I'll get back to this.

@masci masci requested review from anakin87 and removed request for ZanSara November 6, 2023 08:48
@masci
Copy link
Contributor

masci commented Nov 6, 2023

Document refactoring is done, we can pick this up again

@silvanocerza
Copy link
Contributor Author

Pushed with force, previous changes are in elasticsearch-backup branch.

@silvanocerza
Copy link
Contributor Author

Tests might be a bit flaky, I had to rerun them as the Elasticsearch container connection timed out.
Just for future reference.

anakin87
anakin87 reviewed Nov 9, 2023
View reviewed changes
Copy link
Member

@anakin87 anakin87 left a comment

Choose a reason for hiding this comment

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

I took a first look and left some comments.
🙂

document_stores/elasticsearch/pyproject.toml Outdated Show resolved Hide resolved
document_stores/elasticsearch/src/elasticsearch_haystack/document_store.py Outdated Show resolved Hide resolved
document_stores/elasticsearch/src/elasticsearch_haystack/document_store.py Show resolved Hide resolved
document_stores/elasticsearch/src/elasticsearch_haystack/document_store.py Outdated Show resolved Hide resolved
document_stores/elasticsearch/src/elasticsearch_haystack/document_store.py Show resolved Hide resolved
document_stores/elasticsearch/src/elasticsearch_haystack/bm25_retriever.py Outdated Show resolved Hide resolved
document_stores/elasticsearch/tests/test_document_store.py Outdated Show resolved Hide resolved
@anakin87 anakin87 self-requested a review November 10, 2023 13:59
anakin87
anakin87 approved these changes Nov 10, 2023
View reviewed changes
Copy link
Member

@anakin87 anakin87 left a comment

Choose a reason for hiding this comment

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

For the time being, all looks good to me...

(we will iterate on this implementation)

@silvanocerza
Copy link
Contributor Author

LET'S GOOOO! 🚀

@silvanocerza silvanocerza merged commit 0f57ce4 into main Nov 10, 2023
4 checks passed
@silvanocerza silvanocerza deleted the elasticsearch branch November 10, 2023 14:02
ElenaKusevska added a commit to Anant/haystack-core-integrations that referenced this pull request Dec 22, 2023
@ElenaKusevska
Merge pull request deepset-ai#41 from Anant/2.0-retriever
41f1f13 
update retriever
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anakin87 anakin87 anakin87 approved these changes

Assignees

@silvanocerza silvanocerza

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@silvanocerza @masci @anakin87