Skip to content

fix: fix grpc-client streaming#2490

Merged
limingxinleo merged 8 commits intohyperf:masterfrom
Reasno:grpc-client-streaming
Sep 21, 2020
Merged

fix: fix grpc-client streaming#2490
limingxinleo merged 8 commits intohyperf:masterfrom
Reasno:grpc-client-streaming

Conversation

@Reasno
Copy link
Copy Markdown
Member

@Reasno Reasno commented Sep 12, 2020

No description provided.

@Reasno Reasno requested review from huangzhhui and limingxinleo and removed request for huangzhhui September 12, 2020 00:52
@Reasno
Copy link
Copy Markdown
Member Author

Reasno commented Sep 12, 2020

streaming的时候 如果连接断了 swoole/grpc的官方实现是返回false,我这里和官方保持了一致,但是可以考虑是不是抛出异常更好一些?

@Reasno Reasno force-pushed the grpc-client-streaming branch 2 times, most recently from 5b3d8cf to 7ee24a2 Compare September 14, 2020 06:10
@Reasno Reasno force-pushed the grpc-client-streaming branch from 7ee24a2 to b30f33f Compare September 14, 2020 06:22
Message $argument,
$deserialize
$deserialize,
array $metadata = [],
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个 metadata 好像没用到啊。。

是预留的么?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果后面可能有用,就预留放着吧,没必要删掉。。

我看其他几个 request 方法,也都有

* @param mixed $data
*/
public function openStream(string $path, string $data = null): int
public function openStream(string $path, $data = '', string $method = '', bool $usePipelineRead = false): int
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里为啥把 string 约束去掉。。如果说支持输入 null

改成 ?string 是不是会好点

limingxinleo
limingxinleo previously approved these changes Sep 17, 2020
@limingxinleo limingxinleo merged commit 7fc9e40 into hyperf:master Sep 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants