Fix OutStream hooks not being called #1367
Conversation
There was a problem hiding this comment.
Thanks @steovd, do you think this can be related to #1369 (comment)?
I don't think either this patch or #1110 have anything to do with it. The same happens on main even when I revert the only possibly relevant change from #1110. I'm not that familiar with the code though, so I could be wrong. |
|
Just checking in - any update on this? This fix would resolve a frustrating bug I'm currently facing. Thanks! |
| @@ -620,10 +621,19 @@ def flush(self): | |||
| else: | |||
| self._flush() | |||
|
|
|||
| @property | |||
There was a problem hiding this comment.
took me a hot second to figure out why this was being used, but don't have any suggestions on making this clearer.
There was a problem hiding this comment.
It's a bit of a hack, but it ensures the correct hooks are passed without the caller having to do anything extra. But if the maintainers don't like it, they can revert it and schedule the partial manually every time instead.
You can work around this bug by registering your hooks from the background thread. I don't have the code at hand but I think it went something like this: outstream.pub_thread.schedule(functools.partial(outstream.register_hook, myhook)) |
Thanks! I ended up doing something similar, but looks like your version is neater. Adding the hook itself is already a hacky fix to what I am trying to do so I still think this PR is useful, but glad there is a workaround. |
#1110 introduced
OutStreamhooks, which are stored in a thread-local variable and invoked in_flush. However, since_flushusually runs in a background thread, the hooks only work when registered from this thread.This PR makes it so that the hooks are accessed from the thread which calls
flush, which I assume is the expected behavior.