Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.3k
gh-124872: Replace enter/exit events with "switched"#124776
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
8242dce to7606b67Comparee68cb7f tod04d9c0Compared04d9c0 tof3f17b3CompareUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
f3f17b3 tob4cff77CompareUsers want to know when the current context switches to a differentcontext object. Right now this happens when and only when a contextis entered or exited, so the enter and exit events are synonymous with"switched". However, if the changes proposed forpythongh-99633 areimplemented, the current context will also switch for reasons otherthan context enter or exit. Since users actually care about contextswitches and not enter or exit, replace the enter and exit events witha single switched event.The former exit event was emitted just before exiting the context.The new switched event is emitted after the context is exited to matchthe semantics users expect of an event with a past-tense name. Ifusers need the ability to clean up before the switch takes effect,another event type can be added in the future. It is not added herebecause YAGNI.I skipped 0 in the enum as a matter of practice. Skipping 0 makes iteasier to troubleshoot when code forgets to set zeroed memory, and italigns with best practices for other tools (e.g.,https://protobuf.dev/programming-guides/dos-donts/#unspecified-enum).
b4cff77 tod898525CompareThis is sort of reviewed already, per discussion in the original issue, so I'm approving it. |
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.
LGTM on behavioral changes.@1st1 as requested
| casePy_CONTEXT_SWITCHED: | ||
| return"Py_CONTEXT_SWITCHED"; | ||
| default: | ||
| return"?"; |
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.
Actually, please you usePy_UNREACHABLE() here
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.
The ? was@gvanrossum idea in my pr.
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.
Hm, alright, I'm not gonna overturn Guido's review :) I'll push this as is.
843d28f intopython:mainUh oh!
There was an error while loading.Please reload this page.
Thank you Richard! |
…events with "switched" (python#124776)" (python#125513)"This reverts commitd3c82b9.
…4776)Users want to know when the current context switches to a differentcontext object. Right now this happens when and only when a contextis entered or exited, so the enter and exit events are synonymous with"switched". However, if the changes proposed forpythongh-99633 areimplemented, the current context will also switch for reasons otherthan context enter or exit. Since users actually care about contextswitches and not enter or exit, replace the enter and exit events witha single switched event.The former exit event was emitted just before exiting the context.The new switched event is emitted after the context is exited to matchthe semantics users expect of an event with a past-tense name. Ifusers need the ability to clean up before the switch takes effect,another event type can be added in the future. It is not added herebecause YAGNI.I skipped 0 in the enum as a matter of practice. Skipping 0 makes iteasier to troubleshoot when code forgets to set zeroed memory, and italigns with best practices for other tools (e.g.,https://protobuf.dev/programming-guides/dos-donts/#unspecified-enum).
Uh oh!
There was an error while loading.Please reload this page.
Users want to know when the current context switches to a different context object. Right now this happens when and only when a context is entered or exited, so the enter and exit events are synonymous with "switched". However, if the changes proposed forgh-99633 are implemented, the current context will also switch for reasons other than context enter or exit. Since users actually care about context switches and not enter or exit, replace the enter and exit events with a single switched event.
The former exit event was emitted just before exiting the context. The new switched event is emitted after the context is exited to match the semantics users expect of an event with a past-tense name. If users need the ability to clean up before the switch takes effect, another event type can be added in the future. It is not added here because YAGNI.
This commit amends a feature that is new to v3.14 so I did not include a NEWS blurb (theexisting blurb should suffice).
cc@fried
contextvars.Context#99633📚 Documentation preview 📚:https://cpython-previews--124776.org.readthedocs.build/