Skip to content
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

feat(grpc): initialize default metadata #4169

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

waitingsong
Copy link
Member

@waitingsong waitingsong commented Nov 5, 2024

请求中添加 Meta 信息:

  • rpc.method.type: unary|server|client

@codecov-commenter
Copy link

codecov-commenter commented Nov 9, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 93.75000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 85.20%. Comparing base (a3ca53b) to head (57fa626).
Report is 469 commits behind head on main.

Files with missing lines Patch % Lines
packages/grpc/src/comsumer/clients.ts 93.75% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4169      +/-   ##
==========================================
+ Coverage   84.55%   85.20%   +0.65%     
==========================================
  Files         491      519      +28     
  Lines       46778    50128    +3350     
  Branches     5601     5233     -368     
==========================================
+ Hits        39551    42713    +3162     
- Misses       7191     7389     +198     
+ Partials       36       26      -10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@waitingsong waitingsong force-pushed the grpc-method branch 2 times, most recently from 8412bae to b7ca3d3 Compare November 9, 2024 08:11
@waitingsong
Copy link
Member Author

奇怪,啥参数没改,单测就报 Error: No address added out of total 1 resolved errors: [listen EADDRINUSE: address already in use 0.0.0.0:6565] 错误……

@waitingsong waitingsong force-pushed the grpc-method branch 3 times, most recently from 288b3dd to 0662d18 Compare November 9, 2024 14:20
@waitingsong
Copy link
Member Author

终于过了……

let genericFunctionName;
switch (genericFunctionSelector) {
case 0:
options.metadata.set('rpc.method.type', 'unary');
Copy link
Member

@czy88840616 czy88840616 Nov 11, 2024

Choose a reason for hiding this comment

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

这里的 metadata,不能是写死的,要么做到外部可覆盖,要么就搞一个私有的 key 好了,比如叫 midway.rpc.method.type啥的

@@ -1,6 +1,7 @@
import { join } from 'path';

export const grpcServer = {
url: 'localhost:6568',
Copy link
Member

Choose a reason for hiding this comment

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

最好还是新开一个,确保老的不受影响。

@waitingsong
Copy link
Member Author

一折腾又炸了~

@waitingsong waitingsong force-pushed the grpc-method branch 3 times, most recently from 18399b7 to b35590d Compare November 13, 2024 10:22
@waitingsong
Copy link
Member Author

win 下面 jest 调试真麻烦……
vscode 里面还不能直接 npm run test

@czy88840616
Copy link
Member

可以搞个gitpod

} else {
clientOptions.metadata = new Metadata();
}
clientOptions.metadata.set('rpc.definition', definition);
Copy link
Member

Choose a reason for hiding this comment

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

这里有问题吧,如果 clientOptions.metadata 有值,就被覆盖了吧。

这里是不是这样就行了

const meta = new Metadata();
meta.set('xx', xx);
clientOptions.metadata = clientOptions.metadata ? meta.merge(clientOptions.metadata) : meta;

@waitingsong waitingsong force-pushed the grpc-method branch 2 times, most recently from caadbcf to 75e17e6 Compare November 16, 2024 14:29
@@ -71,6 +71,17 @@ export class GRPCClients extends Map {
connectionService[methodName] = (
clientOptions: IClientOptions = {}
) => {
const meta = new Metadata();
meta.set('rpc.definition', definition);
Copy link
Member

@czy88840616 czy88840616 Nov 16, 2024

Choose a reason for hiding this comment

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

话说传递这些有啥用?客户端自己应该都知道吧,不然没法解析出来找到服务了。另外可能也会有性能和安全性问题 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

来都来了~

Copy link
Member

Choose a reason for hiding this comment

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

还是得严谨点。。

key: rpc.method.type
value: unary|server|client
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants