dogpile.cache icon indicating copy to clipboard operation
dogpile.cache copied to clipboard

use TypedDict for backend arguments dictionaries

Open jvanasco opened this issue 5 months ago • 3 comments

I'm in the process of updating some legacy stuff that has been pinned to < 1.10 , when the serialization was decoupled

Re: https://github.com/sqlalchemy/dogpile.cache/pull/191 https://github.com/sqlalchemy/dogpile.cache/commit/21c38da5034d1fafb0b73313540b71d7f785e207

The big issue that lost most of my day: The changelog does not accurately reflect that Redis (and other "Bytes" backends) [should] now use set_serialized instead of set.

While updating, a possible improvement jumped at me - is there any interest in making the config dicts TypedDicts ?

jvanasco avatar Aug 02 '25 00:08 jvanasco

I like typeddicts when they work, but my recollection is they are not very good for a dict that can accept additional keys. if the backend takes arguments and passes them along to the driver without knowing what they are, then I would not want to use typeddict there. you'd need to illustrate where you had in mind for these dicts

zzzeek avatar Aug 03 '25 14:08 zzzeek

That makes sense. I've just been using TypedDicts in the test suites, then type: ignore the extra key on those tests. I just find it a bit easier to catch changes.

So on the redis tests, I have something like:

class ArugumentsDict(TypedDict):
    host: str
    port: int
    db: int
    distributed_lock: NotRequired[bool]
    thread_local_lock: NotRequired[bool]
    lock_timeout: NotRequired[int]
    redis_expiration_time: NotRequired[int]
    blocking_timeout: NotRequired[int]
    lock_class: NotRequired[Any]
    lock_prefix: NotRequired[str]
    redis_expiration_time_hash: NotRequired[Optional[bool]]


class ConfigDict(TypedDict):
    arguments: ArugumentsDict

And then the tests are...

    config_args: ConfigDict = {
        "arguments": {
            "host": REDIS_HOST,
            "port": REDIS_PORT,
            "db": 0,
            "distributed_lock": True,
        }
    }

jvanasco avatar Aug 03 '25 23:08 jvanasco

I'm looking at how we use these arguments right now and it looks like we pull out explicit attributes from them and then use them to construct proper calls to the backend's API, meaning we aren't just passing through like SQLAlchemy client_args or something. and in fact we are not even saving any additional keys in arguments! we throw it away! So in fact you're right these absolutely should be TypedDicts, we dont even have "additional keys".

zzzeek avatar Aug 04 '25 17:08 zzzeek