cccs-eric commented on a change in pull request #3988:
URL: https://github.com/apache/iceberg/pull/3988#discussion_r793812444
##########
File path: python_legacy/iceberg/api/types/conversions.py
##########
@@ -90,11 +99,7 @@ def to_byte_buffer(type_id, value):
@staticmethod
def from_byte_buffer(type_var, buffer_var):
- return Conversions.internal_from_byte_buffer(type_var.type_id,
buffer_var)
-
- @staticmethod
- def internal_from_byte_buffer(type_id, buffer_var):
try:
- return Conversions.from_byte_buff_mapping.get(type_id)(type_id,
buffer_var)
+ return
Conversions.from_byte_buff_mapping.get(type_var.type_id)(type_var, buffer_var)
except KeyError:
- raise TypeError("Cannot deserialize Type: %s" % type_id)
+ raise TypeError("Cannot deserialize Type: %s" % type_var)
Review comment:
Ok, I have changed my mind after make the modifications. The reason is
that for some types like `struct`, the `__str__` method may spit out a very
long string. It goes and [loops over every
field](https://github.com/apache/iceberg/blob/master/python_legacy/iceberg/api/types/types.py#L526)...
For this reason, I would refrain from using `type_var` in the exception
message to keep logs clean. Let me know if you diasgree.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
