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
[BEAM-460] Add size-estimation support to Python SDK Coders #678
[BEAM-460] Add size-estimation support to Python SDK Coders #678
Conversation
R: @silviulica |
@@ -172,6 +202,9 @@ def encode_to_stream(self, value, out, nested): | |||
def decode_from_stream(self, in_stream, nested): | |||
return in_stream.read_bigendian_double() | |||
|
|||
def estimate_size(self, unused_value): |
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.
Can you please explain the 8 here?
LGTM - except a few minor comments |
Thanks, PTAL. |
LGTM |
@charlesccychen Is this ready per our in-person discussion yesterday? |
R: @robertwb Yes, the changes we settled on can use the same interface. |
@@ -63,6 +65,27 @@ def decode(self, encoded): | |||
"""Encodes an object to an unnested string.""" | |||
raise NotImplementedError | |||
|
|||
def estimate_size(self, value): | |||
"""Estimates the encoded size of the given value, in bytes.""" | |||
return len(self.encode(value)) |
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.
The default should use ByteCountingOutputStream, or at least override this in StreamCoderImpl to do so.
Thanks, PTAL. I've addressed your comments. |
LGTM |
8378924
to
015b7e4
Compare
This change facilitates runner support for size-estimation counters.