Add maybe_open_context() an actor wide task-resource cache
#257
Conversation
This file contains 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
tractor/trionics/_mngrs.py
Outdated
| # Further potential examples of interest: | ||
| # https://gist.github.com/njsmith/cf6fc0a97f53865f2c671659c88c1798#file-cache-py-L8 | ||
|
|
||
| class cache: |
There was a problem hiding this comment.
It's unusual to have a lower-case class name, IMO.
There was a problem hiding this comment.
indeed. do you think Cache is ok even though it's never being used as an instance?
There was a problem hiding this comment.
Yeah, it's totally ok.
There was a problem hiding this comment.
we might want to make it _Cache to keep it internal?
goodboy
force-pushed
the
context_caching
branch
2 times, most recently
from
8c6e4ee to
35ac323
Compare
goodboy
force-pushed
the
context_caching
branch
4 times, most recently
from
ef5162a to
52fef7f
Compare
goodboy
force-pushed
the
context_caching
branch
6 times, most recently
from
0bd1be5 to
903ca53
Compare
goodboy
force-pushed
the
context_caching
branch
from
903ca53 to
c747293
Compare
|
@overclockworked64 there I think the test should explain a little better how this works. I also fixed up the naming concern to |
goodboy
force-pushed
the
context_caching
branch
from
c8ad5d2 to
5623daf
Compare
|
gahhhhh windowwwwws |
|
Lol this |
|
If the streaming improvements here seem too lumped in we can also break them out into a separate PR. It might be better to more formally document those fixes and the new test? |
goodboy
force-pushed
the
context_caching
branch
from
83d395c to
a95e04c
Compare
After more extensive testing I realized that keying on the context manager *instance id* isn't going to work since each entering task is going to create a unique key XD Instead pass the manager function as `acm_func` and optionally allow keying the resource on the passed `kwargs` (if hashable) or the `key:str`. Further, pass the key to the enterer task and avoid a separate keying scheme for the manager versus the value it delivers. Don't bother with checking and releasing the lock in `finally:` block, it should be an error if it's still locked.
goodboy
force-pushed
the
context_caching
branch
from
a95e04c to
953d15b
Compare
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.
Like it sounds: a context manager that only opens a new actor-global (aka mem local) resource if one isn't already cached.
This is particularly handy for resources that can be shared by multiple tasks in and actor where the user desires that the lifetime of the resource is maintained until the final user task completes.
Still needs tests obviously.
UPDATE:
This ended up turning into a foray into fixing
MsgStreamclosure/termination semantics after trying to get the original resource caching test working.Todo tests:
triotasks get the same cached value