Skip to content

Conversation

@roblabla
Copy link
Contributor

Currently, when passing a memoryview as an arguments to FromString/parse, an exception may sometimes be raised:

Click me to see stacktrace
File "src/python/grpcio/grpc/_cython/_cygrpc/aio/server.pyx.pxi", line 646, in grpc._cython.cygrpc._handle_exceptions
File "src/python/grpcio/grpc/_cython/_cygrpc/aio/server.pyx.pxi", line 745, in _handle_rpc
File "src/python/grpcio/grpc/_cython/_cygrpc/aio/server.pyx.pxi", line 497, in _handle_unary_unary_rpc
File "src/python/grpcio/grpc/_cython/_cygrpc/aio/common.pyx.pxi", line 37, in grpc._cython.cygrpc.deserialize
File "/src/hl/collector_grpc/crypto_interceptor.py", line 119, in closure
  return inner.request_deserializer(decompressed)
File "/usr/local/lib/python3.8/site-packages/betterproto/__init__.py", line 910, in FromString
  return cls().parse(data)
File "/usr/local/lib/python3.8/site-packages/betterproto/__init__.py", line 873, in parse
  value = self._postprocess_single(
File "/usr/local/lib/python3.8/site-packages/betterproto/__init__.py", line 813, in _postprocess_single
  value = cls().parse(value)
File "/usr/local/lib/python3.8/site-packages/betterproto/__init__.py", line 873, in parse
  value = self._postprocess_single(
File "/usr/local/lib/python3.8/site-packages/betterproto/__init__.py", line 800, in _postprocess_single
  value = value.decode("utf-8")
AttributeError: 'memoryview' object has no attribute 'decode'

I believe this happens when trying to deserialize strings from the memoryview here. This fails because memoryview (like many other byteslike) do not have a decode function. Instead, to create a string from a byteslike, one must use the str(object, encoding) constructor.

Having the ability to pass memoryviews (and other byteslike) to betterproto would be nice, as it would prevent needless copies of bytes each time a subslice is necessary.

Byteslike objects (like memoryview) do not have a decode function defined.
Instead, a string may be created from them by passing them to the str
constructor along with an encoding.
@Gobot1234
Copy link
Collaborator

Should the type hint for parse not be updated to accomodate this change?

@roblabla
Copy link
Contributor Author

Should the type hint for parse not be updated to accomodate this change?

According to the typing docs, bytes is actually a shorthand for typing.ByteString which matches bytes, bytearray and memoryview: https://docs.python.org/3.10/library/typing.html?highlight=typing#typing.ByteString.

And it matches my experience, mypy accepts passing a memoryview to parse without issue.

But I could change the typing to be explicitly typing.ByteString if that's preferable.

@Gobot1234
Copy link
Collaborator

No that's fine then

@Gobot1234 Gobot1234 merged commit 421fdba into danielgtaylor:master Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants