-
Notifications
You must be signed in to change notification settings - Fork 375
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
use direct method replacement for the redis patcher #56
Conversation
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.
Looks good, were we finally able to reproduce the problem and assert this fixes it? If yes, we probably wish to write a test to make sure it stays fixed. Might be complex but still, probably useful as this kind of interaction is hard to spot.
end | ||
end | ||
|
||
@patched = true |
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.
I'd rather leave the @patched = true
section in the caller, as patch_redis_client
is only part of the global patching process, and calling it alone won't patch properly.
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.
That wasn't intended, thank you for pointing it out :)
43d216a
to
31662e1
Compare
Adding ancestors to the redis client class will sometimes lead to infinite recursion if call/call_pipeline have been monkey patched by external code.
31662e1
to
c30be5e
Compare
def call(*args, &block) | ||
@datadog_test_called ||= false | ||
if @datadog_test_called | ||
raise Minitest::Assertion, 'patched methods called in infinite loop' |
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.
👍
Adding ancestors to the redis client class will sometimes lead to
infinite recursion if call/call_pipeline have been monkey patched by
external code.