Skip to content

fix(drivers-memory-conversation-redis): allow an injected client without connection parameters - #2371

Open
snowyukitty wants to merge 1 commit into
griptape-ai:mainfrom
snowyukitty:fix/redis-memory-injected-client
Open

snowyukitty wants to merge 1 commit into
griptape-ai:mainfrom
snowyukitty:fix/redis-memory-injected-client

Conversation

@snowyukitty

Copy link
Copy Markdown

Describe your changes

Passing a configured Redis client to RedisConversationMemoryDriver currently still requires redundant host and port arguments. Allow those arguments to be omitted when a client is supplied, and require them explicitly when the driver creates its own client.

Regression tests cover injected clients with neither, either, or both connection arguments; loading and storing through the supplied client; forwarding connection settings to a newly created client; and rejecting incomplete connection settings before constructing a client.

Validation:

  • Redis, local and Griptape Cloud conversation-memory driver tests plus the memory tests: 72 passed.
  • Ruff lint and format checks on both changed files (lint excludes EXE002 because the Windows mount reports files as executable; both Git modes are 100644).
  • Pyright on the changed driver.
  • The injection regression fails on the upstream base when either connection argument is omitted; supplying both still passes.

Tests ran in an isolated Python 3.12 environment without network access. Redis operations are mocked; no live Redis server was used. The full unit suite was not run.

Issue ticket number and link

Refs #816. This addresses the concrete RedisConversationMemoryDriver example; the issue's broader request about other client-backed drivers remains open.

@codecov

codecov Bot commented Oct 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant