Repository navigation
fix(runtime-sdk): fix contextvars not being shared in WSGI streaming response - #288
ryanking13 wants to merge 2 commits into
Conversation
…treaming response
|
LGTM |
|
✅ Approved — The fix works: one copied context now covers calling the app, iterating it and closing it. The new tests check that values are shared within a stream, kept separate between concurrent streams, and reset when a stream is cancelled. No issues found. The decision is advisory: this comment never approves, blocks or requests changes on the pull request. ✅ All checks passed🔒 Security: ✅ LGTMI found no security concerns. 📖 AGENTS.md: ✅ LGTMThis change does not need any AGENTS.md updates. 📋 Code Quality: ✅ LGTMI found no issues in this change.
⚡ Performance: ✅ LGTMI found no performance concerns. Running each chunk through 📝 Documentation: ✅ LGTMI found no documentation issues. Reviewed commit: |
| # Make sure a single context is used per response. | ||
| # Otherwise, stack switching could cause context variables set by the app | ||
| # to be lost when the iterable is consumed. |
There was a problem hiding this comment.
So to clarify is this a workaround for stack switching being wrong? Or is what is happening the expected behavior?
There was a problem hiding this comment.
It would be nice if the comment would include a position on this. I think this means that our stack switching is still wrong, since the context variables shouldn't change across a stack switch?
fix #287
WSGI streaming response use stack switching. However, they should use the same contextvars, since they are in the same request context.