Skip to content

Conversation

@craig08
Copy link
Contributor

@craig08 craig08 commented Feb 14, 2024

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #12 , #14 , #15, #19

PingXie and others added 7 commits February 8, 2024 22:31
This commit introduces the RedisVectorStore class, extending VectorStore
to enable vector storage and retrieval using Memorystore Redis.
…20)

* fix: fix linter and encoding of MemorystoreChatMessageHistory class.

* fix: use relative path for __init__ imports
* feat(vector store): Add usage demo and enhance Redis integration

- Add Jupyter notebook in `docs/` for using langchain vector store with Memorystore Redis.
- Update `vector_store.py` for improved performance and reliability with Redis.
- Modify `__init__.py` to reflect package structure changes and new functionalities.
- Include `requirements.txt` and `setup.py` for easy installation and dependency management.
- Add `state_of_the_union.txt` in `docs/` as a sample dataset for the notebook demo.

* incorporate review feedback

* added missing license header

* excluded the test file from license check

* reformatted source files

* incorporated review feedback

* fixed lint errors

* fixed more lint errors

* removed key_prefix argument from vectorstore

* incorporated review feedback

* fixed formatting errors

* fixed a bad merge

* suppress mypy errors for setuptools

* trying mypy.ini

* remove setup.py and requirements

* add numpy dependency
* feat: add MemorystoreDocumentSaver class

* fix: use variable in the worker pool of the Cloud Build integration

* fix: update doc saver for json encoding

* fix: align key_prefix behavior

* fix: add back key_prefix if keys_or_ids is given

* fix: fix indent of the comments

* fix: get encoder directly from the Redis client

* fix: add pipeline support for Redis client

* fix: reorder prefix and content_field in constructor

* fix: insert prefix to all given document IDs

* fix: fix tests prefix and chat encoding
@product-auto-label product-auto-label bot added the api: redis Issues related to the googleapis/langchain-google-memorystore-redis-python API. label Feb 14, 2024
@craig08 craig08 changed the title Merge staging into main feat: merge staging (vector store, doc saver) into main Feb 14, 2024
@craig08 craig08 marked this pull request as ready for review February 14, 2024 22:41
@craig08 craig08 requested a review from a team as a code owner February 14, 2024 22:41
craig08 and others added 2 commits February 15, 2024 09:16
…es (#22)

* feat(vector store): Add usage demo and enhance Redis integration

- Add Jupyter notebook in `docs/` for using langchain vector store with Memorystore Redis.
- Update `vector_store.py` for improved performance and reliability with Redis.
- Modify `__init__.py` to reflect package structure changes and new functionalities.
- Include `requirements.txt` and `setup.py` for easy installation and dependency management.
- Add `state_of_the_union.txt` in `docs/` as a sample dataset for the notebook demo.

* incorporate review feedback

* added missing license header

* excluded the test file from license check

* reformatted source files

* incorporated review feedback

* fixed lint errors

* fixed more lint errors

* removed key_prefix argument from vectorstore

* incorporated review feedback

* fixed formatting errors

* fixed a bad merge

* suppress mypy errors for setuptools

* trying mypy.ini

* remove setup.py and requirements

* add numpy dependency

* refactor(vector-store): enhance validation and initialization processes

- Add validation for unsupported data types and negative vector sizes in VectorIndexConfig.
- Refactor field_name parameters in HNSWConfig and FLATConfig to support type hinting with optional strings.
- Enhance RedisVectorStore initialization by enforcing type checks for client, index_name, and embedding_service.
- Introduce an optional 'ids' parameter in methods to allow explicit document identifiers, improving document management.
- Adjust method documentation and error messages for clarity and consistency.

* Incorporated review feedback

* fixed formatting

* incorporated review feedback

* formatting

* fixed bugs

* fix formatting

* added vector store unittests

* actualy added vector store tests

* fix formatting

* fix: fix styles and formats

---------

Co-authored-by: Craig Chi <craigchi@google.com>
@craig08 craig08 changed the title feat: merge staging (vector store, doc saver) into main feat: merge staging (vector store, doc saver/loader) into main Feb 15, 2024
@craig08 craig08 requested a review from averikitsch February 16, 2024 02:16
Copy link
Contributor

@PingXie PingXie left a comment

Choose a reason for hiding this comment

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

LGTM

"""
if ids:
doc_ids = [self._key_prefix + doc_id for doc_id in ids]
self._redis.delete(*doc_ids)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we consider pipelining as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DEL is a single command that accepts multiple keys at once. So I'm not sure how pipeline can help here? I updated it to batch delete though, PTAL, thanks!

@craig08 craig08 requested a review from PingXie February 16, 2024 17:35
@craig08 craig08 merged commit f25d030 into main Feb 16, 2024
@craig08 craig08 deleted the staging branch February 16, 2024 17:47
@release-please release-please bot mentioned this pull request Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: redis Issues related to the googleapis/langchain-google-memorystore-redis-python API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add basic tests for chat message history class

3 participants