Skip to content

Conversation

@yju0808
Copy link
Contributor

@yju0808 yju0808 commented Nov 14, 2025

Fix Redis SSL Connection Error with redis-py

Summary

This PR fixes a TypeError that occurs when using RedisSession.from_url() with SSL-enabled Redis URLs (rediss://) and redis-py 7.x.

Problem

RedisSession.from_url() currently adds ssl=True to kwargs when detecting a rediss:// URL. This causes a TypeError because AbstractConnection.__init__() does not accept the ssl parameter.

Error:

TypeError: AbstractConnection.__init__() got an unexpected keyword argument 'ssl'

Changes

  • Removed: The line that adds ssl=True to redis_kwargs when rediss:// is detected
  • Removed: import of urlparse (no longer used)
  • Reason: In redis-py 7.x, the rediss:// URL scheme automatically triggers SSL handling via parse_url(), making the explicit ssl=True parameter unnecessary and incompatible

Related Issue

Fixes #2081

@yju0808 yju0808 changed the title fix: resolve Redis SSL connection error with openai-agents fix: resolve Redis SSL Connection Error with redis-py 7.x Nov 14, 2025
@yju0808 yju0808 changed the title fix: resolve Redis SSL Connection Error with redis-py 7.x fix: resolve Redis SSL Connection Error with redis-py Nov 14, 2025
Copy link
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

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

To make things simpler, I'd like to change the minimal requreid version of redis client to ">=7". Can you apply the change as well?

@seratch seratch added this to the 0.5.x milestone Nov 15, 2025
@yju0808
Copy link
Contributor Author

yju0808 commented Nov 15, 2025

To make things simpler, I'd like to change the minimal requreid version of redis client to ">=7". Can you apply the change as well?

@seratch Updated!
I’ve updated the minimum required Redis version to >=7 as requested.

Changes included:
-Updated pyproject.toml: redis>=6.4.0 → redis>=7
-Regenerated uv.lock: resolved to redis==7.0.1

Let me know if anything else is needed!

@yju0808 yju0808 requested a review from seratch November 15, 2025 03:43
@seratch seratch enabled auto-merge (squash) November 15, 2025 03:57
@seratch seratch disabled auto-merge November 15, 2025 03:58
Copy link
Member

@seratch seratch left a comment

Choose a reason for hiding this comment

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

Can you fix the mypy errors?

Run make mypy
  make mypy
  shell: /usr/bin/bash -e {0}
  env:
    UV_FROZEN: 1
    UV_CACHE_DIR: /home/runner/work/_temp/setup-uv-cache
uv run mypy . --exclude site
src/agents/extensions/memory/redis_session.py:258: error: Incompatible types in "await" (actual type "Awaitable[bool] | bool", expected type "Awaitable[Any]")  [misc]
tests/extensions/memory/test_redis_session.py:495: error: Incompatible types in "await" (actual type "Awaitable[bool] | bool", expected type "Awaitable[Any]")  [misc]
tests/extensions/memory/test_redis_session.py:502: error: Incompatible types in "await" (actual type "Awaitable[bool] | bool", expected type "Awaitable[Any]")  [misc]
tests/extensions/memory/test_redis_session.py:784: error: Incompatible types in "await" (actual type "Awaitable[bool] | bool", expected type "Awaitable[Any]")  [misc]
Found 4 errors in 2 files (checked 366 source files)
make: *** [Makefile:20: mypy] Error 1
Error: Process completed with exit code 2.

@seratch seratch changed the title fix: resolve Redis SSL Connection Error with redis-py fix: #2081 Redis SSL Connection Error with redis-py v7 Nov 15, 2025
@yju0808
Copy link
Contributor Author

yju0808 commented Nov 15, 2025

@seratch Fixed!

I've resolved all mypy errors by adding # type: ignore[misc] comments to the Redis async ping() calls.

Changes included:
-Added type: ignore[misc] comments for ping() calls in redis_session.py
-Added the same ignores for corresponding assertions in test_redis_session.py

Root cause:
The redis-py library defines return types as Union[Awaitable[T], T] for sync/async compatibility, but mypy cannot infer that redis.asyncio.Redis always returns Awaitable[T] in async context.

Note:
The codebase already uses the same type: ignore[misc] pattern for other Redis async methods (e.g., lrange() and rpop()), so I followed the existing convention for consistency.

@yju0808 yju0808 requested a review from seratch November 15, 2025 04:33
@seratch seratch merged commit 05dc79d into openai:main Nov 15, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Redis SSL Connection Error with redis-py

2 participants