fix json_encode to handle non-string dict keys#10265
Open
Mr-Neutr0n wants to merge 1 commit intodetermined-ai:mainfrom
Open
fix json_encode to handle non-string dict keys#10265Mr-Neutr0n wants to merge 1 commit intodetermined-ai:mainfrom
Mr-Neutr0n wants to merge 1 commit intodetermined-ai:mainfrom
Conversation
the jsonable() helper was only converting dict values but not keys, so bytes keys (e.g. from profiling metrics) would cause a TypeError in json.dumps. now both keys and values get converted. fixes determined-ai#10263
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @Mr-Neutr0n on file. In order for us to review and merge your code, please start the CLA process at https://determined.ai/cla. After we approve your CLA, we will update the contributors list (private) and comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When profiling is enabled, some metrics dicts end up with
byteskeys (from system-level profiling data). Thejsonable()helper inutil.json_encodewas recursing into dict values but passing keys through unchanged, sojson.dumpswould blow up withTypeError: keys must be str, int, float, bool or None, not bytes.The fix is one line — apply
jsonable()to dict keys too, not just values. This way bytes keys get decoded to strings the same way bytes values already do.Also added a test for this case.
Fixes #10263