Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve docstring for emit #1070

Closed
sed-i opened this issue Nov 20, 2023 · 3 comments · Fixed by #1072
Closed

Improve docstring for emit #1070

sed-i opened this issue Nov 20, 2023 · 3 comments · Fixed by #1072
Labels
docs Improvements or additions to documentation

Comments

@sed-i
Copy link
Contributor

sed-i commented Nov 20, 2023

Iiuc, emission of custom events does not get queued: when they are emitted, their handler is called immediately, before the previous hook finished.

operator/ops/framework.py

Lines 331 to 335 in 26c6e95

def emit(self, *args: Any, **kwargs: Any):
"""Emit event to all registered observers.
The current storage state is committed before and after each observer is notified.
"""

I.e. emission is nested, not sequential.

1. Hook 1
2.   Hook 2
3.     Hook 3
4.   Finish hook 2 handler
5. Finish hook 1 handler

Could be handy to have it outlined in the docs.

@benhoyt benhoyt added the docs Improvements or additions to documentation label Nov 20, 2023
@benhoyt
Copy link
Collaborator

benhoyt commented Nov 20, 2023

@sed-i Would you be willing to suggest a concrete docstring change?

@sed-i
Copy link
Contributor Author

sed-i commented Nov 21, 2023

How about:

         """Emit event to all registered observers. 
  
         The current storage state is committed before and after each observer is notified. 
+        Note that emission of custom events is handled immediately. This means that a call to
+        `emit` is followed by immediately calling the observers. In other words, emission of
+        custom events is not queued but rather nested. For example:
+        1. Core hook (emits custom_event_1)
+        2.   Custom event 1 handler (emits custom_event_2)
+        3.     Custom event 2 handler
+        4.   Resume custom event 1 handler
+        5. Resume core hook handler
         """ 

@benhoyt
Copy link
Collaborator

benhoyt commented Nov 21, 2023

Thanks, done in #1072.

I've basically taken your suggestion above, but removed the middle sentence as I thought that was basically repeating the same thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants