fix: Attempting to serialize bytes type causes error (#5660)#5681
fix: Attempting to serialize bytes type causes error (#5660)#5681aviruthen wants to merge 2 commits intoaws:masterfrom
Conversation
mufaddal-rohawala
left a comment
mufaddal-rohawala
left a comment
There was a problem hiding this comment.
🤖 AI Code Review
The fix correctly adds bytes to the primitive type checks to prevent TypeError when serializing bytes objects. However, there are indentation issues in the modified lines that will likely cause linting failures, and there's a minor extra blank line in the test file.
|
|
||
| def is_not_primitive(obj): | ||
| return not isinstance(obj, (int, float, str, bool, datetime.datetime)) | ||
| return not isinstance(obj, (int, float, str, bool, bytes, datetime.datetime)) |
There was a problem hiding this comment.
Bug: This line has 8 spaces of indentation (double-indented) instead of the original 4 spaces. This will cause an IndentationError since the function body was previously at 4-space indentation. Please fix:
return not isinstance(obj, (int, float, str, bool, bytes, datetime.datetime))|
|
||
| def is_primitive_class(cls): | ||
| return cls in (str, int, bool, float, datetime.datetime) | ||
| return cls in (str, int, bool, float, bytes, datetime.datetime) |
There was a problem hiding this comment.
Bug: Same indentation issue here — 8 spaces instead of 4. This will cause an IndentationError. Please fix:
return cls in (str, int, bool, float, bytes, datetime.datetime)| @@ -373,6 +373,25 @@ def test_serialize_method_nested_shape(): | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Nit: There's an extra blank line here. PEP 8 expects two blank lines between top-level definitions, not three. This is minor but worth cleaning up.
aviruthen
deleted the
fix/attempting-to-serialize-bytes-type-causes-error-5660
branch
2 participants
Description
The
serialize()function insagemaker-core/src/sagemaker/core/utils/utils.pyfails when encounteringbytesobjects becausebytesis not included in theis_not_primitive()check. Whenserialize({'body': b'1'})is called, the bytes value passes through to_serialize_shape(), which callsvars()on the bytes object, causing aTypeError. The fix is to addbytesto the primitive type tuple inis_not_primitive()andis_primitive_class()so that bytes values are returned as-is during serialization.Related Issue
Fixes #5660
Changes Made
sagemaker-core/src/sagemaker/core/utils/utils.pysagemaker-core/tests/unit/generated/test_utils.pyAI-Generated PR
This PR was automatically generated by the PySDK Issue Agent.
Merge Checklist
prefix: descriptionformat