-
Notifications
You must be signed in to change notification settings - Fork 157
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
[RFC] Alternative approach to "related events". #240
Conversation
Thanks! I really like this approach and the neat API design. 👍 Maybe one question for a possible backward-compatible opt-in feature: Do you think a simple Let me tinker around with it in @mitmproxy to see how this all works out. But I think this is a good step into the right direction. |
So your two suggestions are slightly different, let's tackle them in turn:
We certainly could do this. I think it's worth considering, in part because it makes #228 dramatically simpler. We could essentially manage this transition like we've managed the transition for #154: add a flag on the connection object that affects this behaviour, and then remove the fallback for 3.0.0. That would then make it much easier to do the lazy iteration behaviour of #228. What's not clear to me yet is whether this transforms the API in a manner that is altogether too expert-oriented. It's definitely a weird subtlety to have events that are only sometimes emitted at the top level, and the rest of the time need to be fished out of other events. We could keep the fallback around, but at that point we basically have two APIs that function entirely differently. Not ideal.
The answer is probably, but it leads to a weird inconsistency with |
Ok, yes I think in that case it makes sense to keep StreamEnded as full event. This avoids confusion. For the flag - I'm in favour of this. Maybe even considering setting it as default in 3.0.0 and for now just as opt-in. 👍 |
Ok, let's do this. I'll aim to push a release this week. |
This is an alternative approach to the "related events" API, as discussed in #234. Another approach is in #238, which should be read for the primary discussion.
This RFC differs from #238 by strictly enumerating the possible related events for each primary event and hanging those events directly off named properties. This API has the advantage that it's extremely explicit: code cannot accidentally misbehave anywhere near as easily as it can with #238. However, it has the disadvantage that it's substantially less extensible: if more related events pop up, it requires additional work to plumb them through in the right places.
Regardless, this is worth looking at. /cc @Kriechi @mhils @python-hyper/contributors