-
Notifications
You must be signed in to change notification settings - Fork 895
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
Make multi-instrument idioms for async callbacks possible #2281
Conversation
I believe we also need to loosen the requirement that APIs produce an error when multiple instruments are registered with the same name. Does that make sense to do in this PR or in a separate one? |
@jack-berg this PR definitely depends on #2270. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for tweaking this @jmacd !
It seems natural that if multiple callbacks allowed, that there should be some way of unregister / cancel / stop callbacks #2232. But I think that can be solved separately. |
Reviewers, as noted in #2226 (comment) I am closing this PR. This work will be integrated into #2270 following the discussion in #2226. |
Fixes #2280
Changes
This changes slightly the requirement for how instruments are associated with callbacks. Whereas before the spec required one instrument per callback, this would allow multiple instruments. A motivating example is given in #2280.
Related issues #2270 #2249