Open theoturner opened 2 years ago
Could you clarify, @theoturner , what the current round-trip behavior is with a simple "echo" server that just returns the received message:
def echo(request: api.MyMessage) -> api.MyMessage):
return request
def test_roundtrip():
sent = api.MyMessage(name='foo', type=api.MyType(0))
received = client.echo(sent)
assert sent == received # ?
assert sent.name == received.name # ?
assert send.type == received.type # ?
assert received.type == api.MyType.A # ?
I get: True True False False
Edit: correction on first line
Thanks for checking. Indeed, that behavior seems "surprising" in best case. @danielgtaylor , what's your appetite for getting this fixed? We're happy to provide the code.
@theoturner I wonder if this got fixed in the meantime? On today's master, the following test case passes: https://github.com/uschi2000/python-betterproto/pull/1/files
@pytest.mark.asyncio
async def test_messages_roundtrip_with_default_fields():
async with ChannelFor([ThingService()]) as channel:
client = ThingServiceClient(channel)
request = DoThingRequest("name", ["comment"], ThingType.UNKNOWN, 0) # UNKNOWN and 0 are protobuf default values
response = await client.do_thing(request)
# Assert we get back the request
assert request == response.request
assert response.names == ["name"]
assert response.request.number == 0
assert response.request.type == ThingType.UNKNOWN
assert id(request) != id(response.request) # prove that objects went through serde
# Try with a newly construction request object
expected_request = DoThingRequest("name", ["comment"], ThingType.UNKNOWN, 0)
assert expected_request == response.request
I'm pretty sure the issue you are describing here is fixed by #293
Protobuf does not serialise default values, however the betterproto objects could retain them.
Consider:
I create a new message with the default enum:
If I send this message, and the receiver creates a betterproto message, they get:
Fortunately betterproto has equality between these messages, even though in the latter type is not specified.
However, this is not particularly easy to find without a bit of digging. I believe it would be simpler to just add the default values when deserialising, which is actually pretty simple:
I'm sure there is a neater way of handling the internal attributes e.g.
_serialise_on_wire
.With the above, the receiver would have the same object as the sender, which is more intuitive.