[WIP] Change cache/factory lookup fallback strategy to fix caching issue

[enkidulan]

WIP, DO NOT MERGE For more detail see issue https://github.com/mmerickel/wired/issues/12

[mmerickel]

It's hitting this block (below) before searching for a factory. It's there intentionally per the comment, so I can't reorder it without a bit more thought which I don't have atm so leaving this comment here in the meantime!

https://github.com/mmerickel/wired/blob/b10827c8b900abd82953863709b7b3d6d8a086bc/src/wired/container.py#L229-L240

[enkidulan]

I can't image any cases where there is a need to have a fall back to a default cache. @mmerickel could you provide some examples? My feeling is that there is too much logic as for a simple caching...

[mmerickel]

The ServiceContainer.set() api is by far the most problematic part of wired imo. The use case is things like pyramid_services [1]. The part that got complicated was that there is no factory defined which means that it's not clear to what context the service should apply. I'm working on a few options but I feel strongly that .set() is a good idea, I'm just not sure I'm approaching it correctly. I'm considering changing it to apply to a specific context object instead of a general interface type (note the default context=Interface). Currently wired properly ignores the context if you lookup a factory and that factory does not want the context. The .set() api is way harder to make work correctly in that case without the fallback idea that is in there now.

[1] https://github.com/mmerickel/pyramid_services/blob/28da85023f1f26cd1b7b9031767a1acfdea81968/src/pyramid_services/__init__.py#L151

[enkidulan]

I've changed the cache/factory lookup fallback strategy to make tests happy, also I've created a _default_context constant. The fallback strategy now looks like:

1. check context cache
2. fallback to context class cache
3. fallback to context iface factory
4. fallback to _default_context cache

And tests are happy but the fallback strategy doesn't seem consistent to me. Would be nice to develop/discuss a good fallback strategy in a separate issue and document it.

[pauleveritt]

I hate to say it but....I wonder if the cache strategy should be pluggable.

I personally hope to have a strategy one day based on ZODB and persistent callables. (Incremental rebuilds for a static site generator.)

[mmerickel]

I hate to say it but....I wonder if the cache strategy should be pluggable.

Wired's caching just about how long wired keeps the services alive on a per-context basis before invoking a factory again to make a new one. If you want to cache some data for a service across context's that's not wired's problem.

[mmerickel]

And tests are happy but the fallback strategy doesn't seem consistent to me. Would be nice to develop/discuss a good fallback strategy in a separate issue and document it.

The only way to implement this correctly right now is to have an override cache configured and I've been spending a lot of time thinking of a better way to do it without coming up with a lot of good ideas so far. The "correct" implementation conceptually is to do something like this:

_override = object()

ctx = get_service(..., context=_override)
if not ctx:
    ctx = get_service(..., context=context)

The drawback here is that it requires a completely separate cache object created, whereas right now I'm re-using the context=None cache which is causing the aliasing issue your tests expose. It also requires 2 lookups for every service, one in context=_override and one in context=context. But does define the behavior I'm going for so I might have to just implement it that way.

[mmerickel]

I replaced this with #14.