readme and param defaults update #3
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tylerhutcherson
added a commit
that referenced
this pull request
Feb 20, 2025
Refactor interfaces related to the Redis client in AsyncRedisIndex (and
possibly others).
## Constraints
- We want to initialize the Redis connection lazily, not at object
instantiation or module import.
- Python doesn't have async properties, so properties are limited to
sync function calls
- Therefore, we don't want properties to be part of the interface
- For functions we "remove," we'll leave them in place with a
deprecation warning
- Remaining backwards-compatible until the next major release
## The Design Today
- Can create a client and set it using `index.connect()`
- Pass in a `redis_url`
- Calls `index.set_client()`
- Can pass in a client and set it with `index.set_client()`
- Pass in a client instance
- **Can't** pass in a client instance with `__init__()`
- Or anything URL, etc.
- Can create a client with `index.from_existing()`
- Pass it a `redis_url`
- Calls `index.set_client()`
- **Can't** use index as an async context manager
- Requires explicit resource handling, `atexit` handler
- But this is broken
- The `setup_async_redis()` decorator creates a function wrapper that
calls `validate_async_redis()` with every `set_client()` call
- The `RedisConnectionFactory.connect()` returns incompatible types
(sync, async `Redis`)
- Sync vs. async depends on a parameter
## The Design After Refactor
- **Can** use as an async context manager
- Either disconnect the index manually with `disconnect()` or use it as
a context manager
- `async with Index()...` will `disconnect()` after you exit the context
block
- Lazily instantiate client with `self._get_client()` method ("private")
- Remove `set_client()` public interface if possible
- Lazily instantiate client
- Remove `connect()` public interface if possible
- Lazily instantiate client
- Leave `from_existing()`
- Leave `client()` property, now just returns `._redis_client` and can
be None
- Call `validate_async_redis()` when setting a new client instance for
the first time
- But make this an internal check rather than attached to public
interface
- Allow `redis_url`, `redis_kwargs`, or `redis_client` in `__init__()`
### Examples Post-Refactor
```python
#1: New instance with redis URL
index = AsyncRedisIndex(redis_url="...")
#2: New instance with existing client
index = AsyncRedisIndex(redis_client=client)
#3: New instance with `from_existing`
index = AsyncRedisIndex.from_existing(..., redis_url="...")
#4 Passing both a client and a URL is isallowed
try:
index = AsyncRedisIndex(redis_client=client, redis_url="...")
except:
pass
else:
raise RuntimeError("Should have raised!")
# The client is lazily connected here, and we send telemetry.
await index.something_using_redis()
await index.something_else_using_redis()
# Close the client.
await index.close()
async with AsyncRedisIndex(redis_url="...") as index:
# Same story: client is lazily created now
await index.something_using_redis()
await index.something_else_using_redis()
# Client is automatically closed
# __enter__ is reentrant
async with index:
# Lazily opens a new client connection.
await index.a_third_thing_using_redis()
# Client is automatically closed
```
---------
Co-authored-by: Tyler Hutcherson <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
pip installand examples in README to point tosample-data/initial_capparam on Vector field