Make some minor improvements for LastValueCache #974
Conversation
When debugging and having some task in the stack coming from a `LastValueCache` it is very difficult to identify which one is it. This commit add a new `unique_id` that's used to create the task name. Signed-off-by: Leandro Lucarella <[email protected]>
github-actions
bot
added
part:data-pipeline
llucax
added
the
cmd:skip-release-notes
|
Enabled auto-merge. |
llucax
added
the
type:tech-debt
|
|
||
| def __str__(self) -> str: | ||
| """Return a string representation of the cache.""" | ||
| return str(self._latest_value) |
There was a problem hiding this comment.
Maybe the docstring should say the latest value in the cache instead?
There was a problem hiding this comment.
OK, I tend to use just a generic docstring for __str__ and __repr__ but maybe in this case it is worth to be more explicit.
|
|
||
| def __del__(self) -> None: | ||
| """Stop the cache when it is deleted.""" | ||
| self._task.cancel() |
There was a problem hiding this comment.
Best to check if the event loop is still running before calling cancel, else, we might get more warning spam in tests.
There was a problem hiding this comment.
Oh, really? Actually I added this because I was getting warnings in tests about the last value cache tasks being pending when the loop was destroyed. But I debugged the issue and the tasks were properly finished, it seems it might be a bug in pytest-async, I believe we had some similar issues in the past.
Maybe I should remove this and that's it, I think according to Python specs this should not be necessary, and tasks should be cancelled (or discarded) when the even loop is closed, and dangling tasks will also be removed from the loop and finished, it might only happen that this is delayed until the GC runs if the reference was part of a reference loop.
I'd say until we are completely sure this is really necessary we just don't add it.
There was a problem hiding this comment.
Just to add more context, this is the issue that made me try this:
The `str` just returns the value, and `repr` gets all the info, including the new `unique_id`. We also implement `__str__` for `_Sentinel` to return a more meaningful string, since it will be shown in the `LatestValueCache` string representation when no value has been received yet. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
llucax
force-pushed
the
minor-improvements
branch
from
d8a9326 to
e9b2e05
Compare
|
Updated to improve the |
2 participants
unique_idtoLatestValueCache__str__and__repr__toLatestValueCacheLatestValueCache's task if deleted@overridetoPowerDistributingActor