Refactor: remove grpc wrapper #1634
Conversation
bfb6974 to
bd8e20b
Compare
|
/retest |
1 similar comment
|
/retest |
| s.Start(endpoint, d, d, d, testBool) | ||
| s.Wait() | ||
| //setup grpc server | ||
| opts := []grpc.ServerOption{ |
There was a problem hiding this comment.
what's the intention for this?
why not wrap NewNonBlockingGRPCServer to make logic more clear?
There was a problem hiding this comment.
the wrapper is not shared among csi projects.
It is only referred by main once.
The wrapper doesn't introduce any extra function which grpc server provides.
There was a problem hiding this comment.
Also, we don't need to inject any code to prod code for test purposes. the metrics related code can be put to one place. mock objects can be injected via constructor which simplify the ut implementation.
and the line of code decreased.
I think this will make the code clearer.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andyzhangx, MartinForReal The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Refactor: remove grpc wrapper
Which issue(s) this PR fixes:
Fixes #
Requirements:
Special notes for your reviewer:
Release note: