-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[DBMON-2435] serialize payload with type bytes in json dumps #15763
Conversation
Codecov Report
Flags with carried forward coverage won't be shown. Click here to find out more. |
def _hash_to_hex(hash): | ||
return to_native_string(binascii.hexlify(hash)) | ||
def _hash_to_hex(hash) -> str: | ||
return binascii.hexlify(hash).decode("utf-8") |
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.
to_native_string
converts bytes b'xxx'
to "b'xxx'"
. The correct str representation of the hash should be properly decoded.
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.
Could we had unit tests to confirm the bug is fixed?
c21d6a4
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! Thanks for adding the tests!
I left one suggestion, but feel free to merge as it is if you want
b80c02e
What does this PR do?
This PR fixes JSON serialization bug
TypeError: Type is not JSON serializable: bytes
in SQL Server activity collection, which cause the check to abruptly shutdown and restart.Motivation
Fixing a long standing bug reported by customer since v7.37.1 which causes the SQL Server activity collection to shutdown and restart.
Additional Notes
Review checklist (to be filled by reviewers)
qa/skip-qa
label if the PR doesn't need to be tested during QA.