Add more log context checks when patching inlineCallbacks#6127
Add more log context checks when patching inlineCallbacks#6127erikjohnston merged 15 commits intodevelopfrom
Conversation
richvdh
left a comment
There was a problem hiding this comment.
looks generally sane, but a few thoughts
| frame.f_code.co_filename, | ||
| ) | ||
| ) | ||
| changes.append(err) |
There was a problem hiding this comment.
why do we put these in an array, rather than printing them out immediately?
There was a problem hiding this comment.
I've added some comments, but broadly functions do change contexts fairly frequently, e.g. when using Measure or manually creating log contexts, so we only really want to log this info if it later turns out something is going wrong.
In future we might be able to do something that lets us turn this into a hard error by somehow understanding when its "ok" that the log context has changed.
| else: | ||
| d = gen.send(result) | ||
| except (StopIteration, defer._DefGen_Return) as e: | ||
| if LoggingContext.current_context() != expected_context: |
There was a problem hiding this comment.
isn't this case handled by check_ctx above?
There was a problem hiding this comment.
(if we're going to double-check the normal-termination case, we should probably also double-check the raised-an-exception case, and tbh it looks like a bunch of cod e that we can get rid of)
There was a problem hiding this comment.
Hmm, yeah, I mainly found it useful for the line numbers
Co-Authored-By: Richard van der Hoff <[email protected]>
This does two things:
Adds a function that tracks changes in log contexts over yield points. This helps track down roughly where things are going wrong.
Adds an env var to enable patching, so that we can enable it for things like sytest as well as unit tests.