TrueCloudLab/frostfs-node
20
1
Fork 27
Code Issues 100 Pull requests 4 Projects Releases 1 Activity Actions

control: use generics for response wrappers #290

Merged
fyrchik merged 1 commit from fyrchik/frostfs-node:control-generics into master 2023-04-28 13:59:44 +00:00
Conversation 0 Commits 1 Files changed 2 +31 -187
2 changed files with 31 additions and 187 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit f73ac6e02d - Show all commits

171
pkg/services/control/convert.go
View file

@ -14,171 +14,26 @@ func (w *requestWrapper) ToGRPCMessage() grpc.Message {
return w.m
}
type healthCheckResponseWrapper struct {
m *HealthCheckResponse
type responseWrapper[T grpc.Message] struct {
ale64bit marked this conversation as resolved
ale64bit commented 2023-04-28 08:08:13 +00:00
Review
Copy link

this is kinda duped now:

Line 17 in ff25521
type responseWrapper[M grpc.Message] struct {


let's move it to a common package and reuse it?

this is kinda duped now: https://git.frostfs.info/TrueCloudLab/frostfs-node/src/commit/ff255212046f508faa637b04d61418bc5bb39fd4/pkg/services/control/ir/convert.go#L17 let's move it to a common package and reuse it?
fyrchik commented 2023-04-28 10:10:30 +00:00
Review
Copy link

My rule of thumb is to abstract something when we have 3 occurences, we have 2 currently :)
The helper looks simple enough to tolerate some amount of duplication.

My rule of thumb is to abstract something when we have 3 occurences, we have 2 currently :) The helper looks simple enough to tolerate some amount of duplication.
ale64bit commented 2023-04-28 10:37:24 +00:00
Review
Copy link

As you wish.

IMHO, abstracting it is trivial (move it to a separate package) and it's reasonable to expect that the abstraction will be used more than two places, since it serves to write many wrappers of the same type which are prevalent in the codebase. What's the benefit of tolerating the duplication anyway?

Could you elaborate how you came to the constant 3 in your rule of thumb? I'm curious about the rationale behind it.

As you wish. IMHO, abstracting it is trivial (move it to a separate package) and it's reasonable to expect that the abstraction will be used more than two places, since it serves to write many wrappers of the same type which are prevalent in the codebase. What's the benefit of tolerating the duplication anyway? Could you elaborate how you came to the constant 3 in your rule of thumb? I'm curious about the rationale behind it.
fyrchik commented 2023-04-28 10:40:32 +00:00
Review
Copy link

We will simplify the api-go itself in the near future, as @dstepanov-yadro noticed, these wrappers look like a kludge.

We will simplify the api-go itself in the near future, as @dstepanov-yadro noticed, these wrappers look like a kludge.
message *T
}
func (w *healthCheckResponseWrapper) ToGRPCMessage() grpc.Message {
return w.m
func newResponseWrapper[T grpc.Message]() *responseWrapper[T] {
return &responseWrapper[T]{
message: new(T),
}
}
func (w *healthCheckResponseWrapper) FromGRPCMessage(m grpc.Message) error {
var ok bool
func (w *responseWrapper[T]) ToGRPCMessage() grpc.Message {
return w.message
}
w.m, ok = m.(*HealthCheckResponse)
func (w *responseWrapper[T]) FromGRPCMessage(m grpc.Message) error {
response, ok := m.(*T)
if !ok {
return message.NewUnexpectedMessageType(m, w.m)
return message.NewUnexpectedMessageType(m, w.message)
}
return nil
}
type setNetmapStatusResponseWrapper struct {
message.Message
m *SetNetmapStatusResponse
}
func (w *setNetmapStatusResponseWrapper) ToGRPCMessage() grpc.Message {
return w.m
}
func (w *setNetmapStatusResponseWrapper) FromGRPCMessage(m grpc.Message) error {
var ok bool
w.m, ok = m.(*SetNetmapStatusResponse)
if !ok {
return message.NewUnexpectedMessageType(m, w.m)
}
return nil
}
type dropObjectsResponseWrapper struct {
message.Message
m *DropObjectsResponse
}
func (w *dropObjectsResponseWrapper) ToGRPCMessage() grpc.Message {
return w.m
}
func (w *dropObjectsResponseWrapper) FromGRPCMessage(m grpc.Message) error {
var ok bool
w.m, ok = m.(*DropObjectsResponse)
if !ok {
return message.NewUnexpectedMessageType(m, w.m)
}
return nil
}
type listShardsResponseWrapper struct {
m *ListShardsResponse
}
func (w *listShardsResponseWrapper) ToGRPCMessage() grpc.Message {
return w.m
}
func (w *listShardsResponseWrapper) FromGRPCMessage(m grpc.Message) error {
var ok bool
w.m, ok = m.(*ListShardsResponse)
if !ok {
return message.NewUnexpectedMessageType(m, w.m)
}
return nil
}
type setShardModeResponseWrapper struct {
m *SetShardModeResponse
}
func (w *setShardModeResponseWrapper) ToGRPCMessage() grpc.Message {
return w.m
}
func (w *setShardModeResponseWrapper) FromGRPCMessage(m grpc.Message) error {
var ok bool
w.m, ok = m.(*SetShardModeResponse)
if !ok {
return message.NewUnexpectedMessageType(m, w.m)
}
return nil
}
type synchronizeTreeResponseWrapper struct {
*SynchronizeTreeResponse
}
func (w *synchronizeTreeResponseWrapper) ToGRPCMessage() grpc.Message {
return w.SynchronizeTreeResponse
}
func (w *synchronizeTreeResponseWrapper) FromGRPCMessage(m grpc.Message) error {
r, ok := m.(*SynchronizeTreeResponse)
if !ok {
return message.NewUnexpectedMessageType(m, (*SynchronizeTreeResponse)(nil))
}
w.SynchronizeTreeResponse = r
return nil
}
type evacuateShardResponseWrapper struct {
*EvacuateShardResponse
}
func (w *evacuateShardResponseWrapper) ToGRPCMessage() grpc.Message {
return w.EvacuateShardResponse
}
func (w *evacuateShardResponseWrapper) FromGRPCMessage(m grpc.Message) error {
r, ok := m.(*EvacuateShardResponse)
if !ok {
return message.NewUnexpectedMessageType(m, (*EvacuateShardResponse)(nil))
}
w.EvacuateShardResponse = r
return nil
}
type flushCacheResponseWrapper struct {
*FlushCacheResponse
}
func (w *flushCacheResponseWrapper) ToGRPCMessage() grpc.Message {
return w.FlushCacheResponse
}
func (w *flushCacheResponseWrapper) FromGRPCMessage(m grpc.Message) error {
r, ok := m.(*FlushCacheResponse)
if !ok {
return message.NewUnexpectedMessageType(m, (*FlushCacheResponse)(nil))
}
w.FlushCacheResponse = r
return nil
}
type doctorResponseWrapper struct {
*DoctorResponse
}
func (w *doctorResponseWrapper) ToGRPCMessage() grpc.Message {
return w.DoctorResponse
}
func (w *doctorResponseWrapper) FromGRPCMessage(m grpc.Message) error {
r, ok := m.(*DoctorResponse)
if !ok {
return message.NewUnexpectedMessageType(m, (*DoctorResponse)(nil))
}
w.DoctorResponse = r
w.message = response
return nil
}

47
pkg/services/control/rpc.go
View file

@ -25,10 +25,7 @@ func HealthCheck(
req *HealthCheckRequest,
opts ...client.CallOption,
) (*HealthCheckResponse, error) {
wResp := &healthCheckResponseWrapper{
m: new(HealthCheckResponse),
}
wResp := newResponseWrapper[HealthCheckResponse]()
wReq := &requestWrapper{
m: req,
}
@ -38,7 +35,7 @@ func HealthCheck(
return nil, err
}
return wResp.m, nil
return wResp.message, nil
}
// SetNetmapStatus executes ControlService.SetNetmapStatus RPC.
@ -47,9 +44,7 @@ func SetNetmapStatus(
req *SetNetmapStatusRequest,
opts ...client.CallOption,
) (*SetNetmapStatusResponse, error) {
wResp := &setNetmapStatusResponseWrapper{
m: new(SetNetmapStatusResponse),
}
wResp := newResponseWrapper[SetNetmapStatusResponse]()
wReq := &requestWrapper{
m: req,
@ -60,7 +55,7 @@ func SetNetmapStatus(
return nil, err
}
return wResp.m, nil
return wResp.message, nil
}
// DropObjects executes ControlService.DropObjects RPC.
@ -69,9 +64,7 @@ func DropObjects(
req *DropObjectsRequest,
opts ...client.CallOption,
) (*DropObjectsResponse, error) {
wResp := &dropObjectsResponseWrapper{
m: new(DropObjectsResponse),
}
wResp := newResponseWrapper[DropObjectsResponse]()
wReq := &requestWrapper{
m: req,
@ -81,7 +74,7 @@ func DropObjects(
return nil, err
}
return wResp.m, nil
return wResp.message, nil
}
// ListShards executes ControlService.ListShards RPC.
@ -90,9 +83,7 @@ func ListShards(
req *ListShardsRequest,
opts ...client.CallOption,
) (*ListShardsResponse, error) {
wResp := &listShardsResponseWrapper{
m: new(ListShardsResponse),
}
wResp := newResponseWrapper[ListShardsResponse]()
wReq := &requestWrapper{
m: req,
@ -102,7 +93,7 @@ func ListShards(
return nil, err
}
return wResp.m, nil
return wResp.message, nil
}
// SetShardMode executes ControlService.SetShardMode RPC.
@ -111,9 +102,7 @@ func SetShardMode(
req *SetShardModeRequest,
opts ...client.CallOption,
) (*SetShardModeResponse, error) {
wResp := &setShardModeResponseWrapper{
m: new(SetShardModeResponse),
}
wResp := newResponseWrapper[SetShardModeResponse]()
wReq := &requestWrapper{
m: req,
@ -123,12 +112,12 @@ func SetShardMode(
return nil, err
}
return wResp.m, nil
return wResp.message, nil
}
// SynchronizeTree executes ControlService.SynchronizeTree RPC.
func SynchronizeTree(cli *client.Client, req *SynchronizeTreeRequest, opts ...client.CallOption) (*SynchronizeTreeResponse, error) {
wResp := &synchronizeTreeResponseWrapper{new(SynchronizeTreeResponse)}
wResp := newResponseWrapper[SynchronizeTreeResponse]()
wReq := &requestWrapper{m: req}
err := client.SendUnary(cli, common.CallMethodInfoUnary(serviceName, rpcSynchronizeTree), wReq, wResp, opts...)
@ -136,12 +125,12 @@ func SynchronizeTree(cli *client.Client, req *SynchronizeTreeRequest, opts ...cl
return nil, err
}
return wResp.SynchronizeTreeResponse, nil
return wResp.message, nil
}
// EvacuateShard executes ControlService.EvacuateShard RPC.
func EvacuateShard(cli *client.Client, req *EvacuateShardRequest, opts ...client.CallOption) (*EvacuateShardResponse, error) {
wResp := &evacuateShardResponseWrapper{new(EvacuateShardResponse)}
wResp := newResponseWrapper[EvacuateShardResponse]()
wReq := &requestWrapper{m: req}
err := client.SendUnary(cli, common.CallMethodInfoUnary(serviceName, rpcEvacuateShard), wReq, wResp, opts...)
@ -149,12 +138,12 @@ func EvacuateShard(cli *client.Client, req *EvacuateShardRequest, opts ...client
return nil, err
}
return wResp.EvacuateShardResponse, nil
return wResp.message, nil
}
// FlushCache executes ControlService.FlushCache RPC.
func FlushCache(cli *client.Client, req *FlushCacheRequest, opts ...client.CallOption) (*FlushCacheResponse, error) {
wResp := &flushCacheResponseWrapper{new(FlushCacheResponse)}
wResp := newResponseWrapper[FlushCacheResponse]()
wReq := &requestWrapper{m: req}
err := client.SendUnary(cli, common.CallMethodInfoUnary(serviceName, rpcFlushCache), wReq, wResp, opts...)
@ -162,12 +151,12 @@ func FlushCache(cli *client.Client, req *FlushCacheRequest, opts ...client.CallO
return nil, err
}
return wResp.FlushCacheResponse, nil
return wResp.message, nil
}
// Doctor executes ControlService.Doctor RPC.
func Doctor(cli *client.Client, req *DoctorRequest, opts ...client.CallOption) (*DoctorResponse, error) {
wResp := &doctorResponseWrapper{new(DoctorResponse)}
wResp := newResponseWrapper[DoctorResponse]()
wReq := &requestWrapper{m: req}
err := client.SendUnary(cli, common.CallMethodInfoUnary(serviceName, rpcDoctor), wReq, wResp, opts...)
@ -175,5 +164,5 @@ func Doctor(cli *client.Client, req *DoctorRequest, opts ...client.CallOption) (
return nil, err
}
return wResp.DoctorResponse, nil
return wResp.message, nil
}