Skip to content

fix: Remove unnecessary connection test and add close/close_async methods to WeaviateDocumentStore#2891

Merged
anakin87 merged 5 commits intodeepset-ai:mainfrom
maxdswain:Weaviate-connection
Mar 6, 2026
Merged

fix: Remove unnecessary connection test and add close/close_async methods to WeaviateDocumentStore#2891
anakin87 merged 5 commits intodeepset-ai:mainfrom
maxdswain:Weaviate-connection

Conversation

@maxdswain
Copy link
Copy Markdown
Contributor

Related Issues

Proposed Changes:

  • Weaviate's python clients now automatically attempt to connect when self._client.connect()/self._async_client.connect() is called allowing both self._client.collections.list_all(simple=True) and self._async_client.collections.list_all(simple=True) to be removed.
  • Add close and async_close methods to WeaviateDocumentStore mirroring what is done in ValkeyDocumentStore and PineconeDocumentStore.
  • Updated relevant tests/fixtures to properly close connections to Weaviate.

How did you test it?

Modified existing integration tests.

Notes for the reviewer

Checklist

@maxdswain maxdswain requested a review from a team as a code owner February 27, 2026 17:19
@maxdswain maxdswain requested review from bogdankostic and removed request for a team February 27, 2026 17:19
@maxdswain maxdswain changed the title fix: Remove unnecessary connection test and add close/close_async methods to WeaviateDocumentStore fix: Remove unnecessary connection test and add close/close_async methods to WeaviateDocumentStore Feb 27, 2026
@github-actions github-actions Bot added the type:documentation Improvements or additions to documentation label Feb 27, 2026
@anakin87 anakin87 requested review from anakin87 and removed request for bogdankostic March 3, 2026 09:36
Copy link
Copy Markdown
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.

Thank you for the contribution!

I left some comments.

Comment thread integrations/weaviate/tests/test_document_store.py
)
yield store
store.client.collections.delete(collection_settings["class"])
store.close()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is this needed?

Copy link
Copy Markdown
Contributor Author

@maxdswain maxdswain Mar 5, 2026

Choose a reason for hiding this comment

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

A lot of the async tests call write_documents which uses the sync client so I think it is yes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, I see. This document store does not have the write_documents_async method...
I'll open an issue to fill this gap.

Comment thread integrations/weaviate/tests/test_document_store_async.py
@maxdswain
Copy link
Copy Markdown
Contributor Author

Thanks for the feedback and review @anakin87, it's much appreciated I'll make sure to be more thorough in the tests I add in future PRs. I believe I have addressed all of your comments now :)

@@ -1,4 +1,3 @@
version: '3.4'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would not remove this line, unless it's needed for some reason

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah I see it's now an obsolete field that can be safely removed

@anakin87 anakin87 self-requested a review March 6, 2026 06:46
Copy link
Copy Markdown
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.

Thank you. Looks good!

I'll make sure to be more thorough in the tests I add in future PRs.

I appreciate your contributions and interactions: it's no small thing in these times dominated by AI agents 🙂

@anakin87 anakin87 merged commit 11a30d6 into deepset-ai:main Mar 6, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration:weaviate type:documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Weaviate: Improve connection check

2 participants