making either store or config

[wasperen2]

fixes #39

[Tinitto]

I guess this is no longer necessary to fix #29 since we will have only one way of initializing the redis store as shown by #35

[Tinitto]

This seems to reintroduce the ambiguity we removed when we removed the redis_store argument.

For instance someone might set both redis_config and redis_store with different redis databases.
It is not quickly apparent to anyone that the redis_store has higher precedence over the redis_config. This might cause that person a lot of frustration when they don't see the data where they expect.

We need to have a single way of passing the redis instance so that there is never a possibility of passing two redis instances.

Options:

1. Extend RedisConfig to allow passing a pre-constructed redis instance.
   Con: It might require a lot of complicated code to ensure two instances are never passed to it (the RedisConfig).
   Pro: The signature of AbstractStore is untouched; no breaking change.
2. Change redis_config argument to redis_store: Union[Redis, AioRedis, RedisConfig].
   Con: The signature of AbstractStore is changed; this is a breaking change.
   Pro: The code is simple and ensures that only one redis instance is ever passed to the store.
   Pro: The name redis_store as opposed to redis_config makes intuitive sense for a parameter that can either be a redis instance or a RedisConfig
3. Change redis_config argument to redis_config: Union[Redis, AioRedis, RedisConfig].
   Pro: The signature of AbstractStore is unchanged; no breaking change.
   Pro: The code is simple and ensures that only one redis instance is ever passed to the store.
   Con: The name redis_config as opposed to redis_store does not make it apparent it can either be a redis instance or a RedisConfig especially since there is an actual class called "RedisConfig"

...There could be other options but personally, I am leaning more on option 3 then adding some extra documentation specifying that redis_config can also receive a redis instance. This way, the many applications currently running out there that were built basing on our original documentation do not fail suddenly on upgrading.

I also expect that not many people except the "power users" (as you commented earlier) will want to have a way of passing in their pre-constructed redis instance. And since they are "power users", they definitely can comb through the documentation for such finer control options :-)

[wasperen2]

I would suggest to stick with the names you have: redis_config and redis_store. The validator does this:

    @model_validator(mode='after')
    def _config_or_redis(self):
        if (self.redis_config is None) and (self.redis_store is None):
            raise ValueError("Must provide one of redis_config or redis_store")
        if (self.redis_config is not None) and (self.redis_store is not None):
            raise ValueError("Cannot provide both redis_config and redis_store")

So if a programmer happens to pass in values for both the redis_config and the redis_store, they will be given a ValueError, explaining they can only pass in one of them.

This way the parameter names remain descriptive.

I will add a docstring that explains this better.

[wasperen2]

Just added the following DocString:

    def __init__(
        self,
        name: str,
        redis_config: Optional[RedisConfig] = None,
        redis_store: Optional[Union[Redis, AioRedis]] = None,
        life_span_in_seconds: Optional[int] = None,
        **data: Any,
    ):
        """
        A store provides all functionality required to quickly persist and retrieve
        pydantic objects into and from a Redis store.

        This store will accept the configuration (in the form of a `RedisConfig`, and
        create a new Redis instance according to that configuration. Alternatively,
        one can pass an existing Redis instance to be used. One can only pass one ot
        these.

        :param name: The name of the store.
        :param redis_config: A Redis configuration object. This will be used to create a
        new Redis instance, using the parameters configured.
        :param redis_store: An existing Redis instance to use. Can only be passed when no
        `redis_config` is passed.
        :param life_span_in_seconds: The default lifespan in seconds that the store will use
        :param data: an other keyword argument that will be passed to the constructor of
        the object
        """

[wasperen2]

This validator is demonstrated to work in the test test_cannot_provide_both as follows:

def test_cannot_provide_both(unused_tcp_port: int):
    redis = redislite.Redis(host="localhost", port=unused_tcp_port)
    config = RedisConfig(host="localhost", port=unused_tcp_port)

    with pytest.raises(ValueError):
        _ = Store(
            name="redis-or-config",
            redis_config=config,
            redis_store=redis,
        )

[Tinitto]

We need the library to be straight forward. Adding two ways of initializing the Store needs to be last resort.
For now, just extend the Store

class MyStore(AbstractStore): # or MyStore(syncio.Store): or or MyStore(asyncio.Store)
    """My custom store"""

    def __init__(
        self, name: str, redis_config: RedisConfig, redis_store: Redis, **data: Any
    ):
        self._redis_store = redis_store
        super().__init__(name, redis_config, **data)

    def _connect_to_redis(self) -> redis.Redis:
        """Connects the store to redis.

        See base class.
        """
        return self._redis_store


store = MyStore(
    name="foobar", redis_config=RedisConfig(), redis_store=your_redis_instance
)

[Tinitto]

This reintroduces the ambiguity that was removed when we removed the redis_store argument. See my earlier comment for some options.

We will just allow users to extend the Store (or AbstractStore) class