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

refactor!: improve the deserialization logic for components that use a Document Store #6466

Merged
merged 12 commits into from
Dec 4, 2023

Conversation

anakin87
Copy link
Member

@anakin87 anakin87 commented Dec 1, 2023

Related Issues

Proposed Changes:

  • Improve the deserialization logic for components that use a Document Store.
    It will no longer be necessary to first import the Document Store used to deserialize Pipelines without errors.

  • Remove the @document_store decorator and the registry of Document Stores.

How did you test it?

CI, adapted the tests.

Notes for the reviewer

After this change, we should remove the decorator from the Document Stores in haystack-core-integrations.

Checklist

@github-actions github-actions bot added topic:tests type:documentation Improvements on the docs labels Dec 1, 2023
@anakin87 anakin87 changed the title Remove docstore decorator and fix import refactor!: improve the deserialization logic for components that use a Document Store Dec 3, 2023
@anakin87 anakin87 marked this pull request as ready for review December 3, 2023 22:01
@anakin87 anakin87 requested review from a team as code owners December 3, 2023 22:01
@anakin87 anakin87 requested review from dfokina and masci and removed request for a team December 3, 2023 22:01
@anakin87 anakin87 self-assigned this Dec 3, 2023
@silvanocerza
Copy link
Contributor

Can we maybe remove the decorator in a later PR?
Otherwise we're going to break all the Document Stores already published, would be great if first we take care of removing the decorator from those and then actually remove it.

@anakin87
Copy link
Member Author

anakin87 commented Dec 4, 2023

Can we maybe remove the decorator in a later PR? Otherwise we're going to break all the Document Stores already published, would be great if first we take care of removing the decorator from those and then actually remove it.

I removed the decorator from the Document Stores in deepset-ai/haystack-core-integrations#76
I also published new releases for Elasticsearch, Opensearch and Chroma Document Stores.

Copy link
Contributor

@masci masci left a comment

Choose a reason for hiding this comment

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

Good to go, I would probably isolate the module-loading logic in an utility method at some point but let's merge

@anakin87 anakin87 merged commit 4912f7c into main Dec 4, 2023
18 checks passed
@anakin87 anakin87 deleted the doc-writer-deserialization branch December 4, 2023 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic:tests type:documentation Improvements on the docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pipeline deserialization fails if the Document Store has not been previously imported
3 participants