using any passed Redis instance

[wasperen2]

Describe the bug One can pass a Redis instance when a new Store is created. And, although self.redis_store is initially set to that value, it is brutally overwritten by a new Redis instance created from the passed config.

This makes it harder to pass in a Mock Redis instance for creating unit tests. It is also unexpected behavior.

To Reproduce

import redis

from pydantic_redis import RedisConfig, Store

my_redis_client = redis.Redis(host='localhost', port=6379, db=0)

redis_config = RedisConfig(host='localhost', port=6379, db=1)

store = Store(
    name="my_redis_store",
    redis_config=redis_config,
    redis_store=my_redis_client,
)

assert store.redis_store == my_redis_client, "Unexpectedly the Redis store I passed is not used"

Expected behavior I would expect the Redis store that I passed in, to be used rather than another one.

[Tinitto]

Oops 🙊. We probably should have made the redis_store property a read-only property. The expectation was the RedisConfig was the only way to initialize the redis instance.

[Tinitto]

For passing a mock redis instance, @wasperen2 , in your use case, are you able to use the way it was done in the test/conftest.py file?

i.e.

• Set aside an unused port
• Create a fake redis instance using "redislite" running on that port
• Initialize the Store or AsyncStore with its "RedisConfig.port" equal to the port from step one

[Tinitto]

Closed this as fixed by the another pull request, assuming the solution provided in the previous comment handles @wasperen2 use case

[wasperen2]

As per my comments, can we please reconsider this and stick with the option to do Dependency Injection (https://en.wikipedia.org/wiki/Dependency_injection)? Having that ability makes your package play better with larger systems where injecting core services is common. This results in more cohesive code and a more standard way of deploying and testing.

[wasperen2]

(it would also not introduce a breaking change :))

[Tinitto]

@wasperen2 I think we should close the current issue first, which is the ambiguity problem. #35 closes this sufficiently because it is in line with the original idea of how the store were to be initialized (as can be seen in all current examples and tests).

We can then take your suggestion for ability to pass in a pre-constructed redis instance as new feature request. If you haven't already, please create a new issue with this feature request so that we can weigh its return-on-investment.