Make notify thread-safe #3275
Make notify thread-safe #3275
Conversation
willmcgugan
requested review from
davep,
rodrigogiraoserrao and
darrenburns
|
|
||
| def unnotify(self, notification: Notification, refresh: bool = True) -> None: | ||
| def _unnotify(self, notification: Notification, refresh: bool = True) -> None: |
There was a problem hiding this comment.
I've never used this API but this method seems like it's really useful if it's public?
Is this method how we discard notifications manually? It seems useful if notifications are longer-lived - to allow us to remove ones that are no longer relevant.
There was a problem hiding this comment.
The problem is not that self.notify() doesn't add the notification immediately. It posts a message, and returns None. That means that the user won't have a reference to the Notification object. And even if they did, it might not yet be active.
Programatically removing notifications is useful, but we will need another mechanism for that.
There was a problem hiding this comment.
Not saying it's for this PR, but I think it'd be nice if notify still returned a Notification as a handle to a notification which may or may not be active. Then, if you try to unnotify a Notification that isn't active then it'd just prevent it from appearing.
Co-authored-by: Dave Pearson <[email protected]>
Makes
notifythread-safe. This is a convenience when working with threaded workers.Fixes #3267