wechaty / grpc

gRPC Service & Protocol Buffers for Wechaty Puppet
https://wechaty.github.io/grpc/
Apache License 2.0
25 stars 22 forks source link

v0.16.2 and v0.16.1 room_qr_code function parameter missing #74

Closed why2lyj closed 3 years ago

why2lyj commented 4 years ago

I'm using python. so pip install chatie-grpc is necessary.

here's different between v0.16.1 and v0.16.2:

chatie_grpc-0.16.2 funcation: async def room_qr_code(self) -> RoomQrCodeResponse:

chatie_grpc-0.16.1 function: async def room_q_r_code(self, *, id: str = "") -> RoomQRCodeResponse:

the latest version(0.16.2) missing parameter id

that me know the consider of removing parameter id for this function, or it's a bug. tks.

huan commented 4 years ago

Thank you very much for the reporting, we will investigate it later.

wj-Mcat commented 4 years ago

@why2lyj I think we should keep chatie_grpc-0.16.1 so that we can push the progress forward.

why2lyj commented 4 years ago

@huan Hi zhuoheng, chatie-grpc 0.17.1 is coming soon, would you pls let me know how about this issue?

huan commented 4 years ago

This issue should be fixed because it's a bug.

Would you like to submit a PR to resolve it?

why2lyj commented 4 years ago

No idea how to fix it :face_with_head_bandage:

huan commented 4 years ago

Yes, to be honest, I have no idea how to fix it either.

Will find some time next week to see what's the problem with this new version of the python grpc generator.

huan commented 3 years ago

@wj-Mcat Could you please help us to confirm that whether this issue still exists in our latest grpc code base? Thank you very much!

wj-Mcat commented 3 years ago

@huan yes, it's a bug for chatie-grpc, which has no id parameter in room_qr_code methods. I will try to fix it later.

huan commented 3 years ago

It's great to know that you will fix it!

I'm wondering if this problem will affect the Python Wechaty because now we are using the latest version of grpc.

Looking forward to your fix, appreciate it!

wj-Mcat commented 3 years ago

It's strange that the generated room_qr_code method has no id parameter. The source definition is :

message RoomQRCodeRequest {
  string id = 1;
}
message RoomQRCodeResponse {
  string qrcode = 1;
}
rpc RoomQRCode (puppet.RoomQRCodeRequest) returns (puppet.RoomQRCodeResponse) {}

And the generated python code is :

async def room_qr_code(self) -> RoomQrCodeResponse:
    request = RoomQrCodeRequest()

    return await self._unary_unary(
        "/wechaty.Puppet/RoomQRCode", request, RoomQrCodeResponse
    )

But, when I change the RoomQRCodeRequest to RoomQrCodeRequest, and change RoomQRCodeResponse to RoomQrCodeResponse, the generated python code is correct :

async def room_qr_code(self, *, id: str = "") -> RoomQrCodeResponse:
    request = RoomQrCodeRequest()
    request.id = id

    return await self._unary_unary(
        "/wechaty.Puppet/RoomQRCode", request, RoomQrCodeResponse
    )

And I think this is a bug in betterproto. @huan

wj-Mcat commented 3 years ago

@huan But when I reduce the version of betterproto to 1.2.5, the generated python code is correct. It works fine.

But, it seems that you betterproto>=2.0.0b1 can fix [import #46 ]() issues.

huan commented 3 years ago

Thanks for the fix!

So it seems that this issue was introduced by a buggy version of better protocol.

Let's keep an eye on this issue and make sure it has been fixed by your latest PR.