Skip to content

Commit

Permalink
[history] fix generated timeout wrapper (cadence-workflow#5737)
Browse files Browse the repository at this point in the history
What changed?

cadence-workflow#5728 code generated history client should exclude some endpoints.

Why?

Some replication endpoints don't have timeout before

How did you test it?

unit test
  • Loading branch information
shijiesheng authored Mar 7, 2024
1 parent 82f08a8 commit e198ca3
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 71 deletions.
2 changes: 1 addition & 1 deletion client/history/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import (
//go:generate gowrap gen -g -p . -i Client -t ../templates/errorinjectors.tmpl -o ../wrappers/errorinjectors/history_generated.go -v client=History
//go:generate gowrap gen -g -p . -i Client -t ../templates/grpc.tmpl -o ../wrappers/grpc/history_generated.go -v client=History -v package=historyv1 -v path=github.com/uber/cadence/.gen/proto/history/v1 -v prefix=History
//go:generate gowrap gen -g -p . -i Client -t ../templates/thrift.tmpl -o ../wrappers/thrift/history_generated.go -v client=History -v prefix=History
//go:generate gowrap gen -g -p . -i Client -t ../templates/timeout.tmpl -o ../wrappers/timeout/history_generated.go -v client=History
//go:generate gowrap gen -g -p . -i Client -t ../templates/timeout.tmpl -o ../wrappers/timeout/history_generated.go -v client=History -v exclude=GetReplicationMessages|GetDLQReplicationMessages|CountDLQMessages|ReadDLQMessages|PurgeDLQMessages|MergeDLQMessages|GetCrossClusterTasks|GetFailoverInfo

// Client is the interface exposed by history service client
type Client interface {
Expand Down
11 changes: 6 additions & 5 deletions client/templates/timeout.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import (
"time"
)

{{$exclude := splitList "|" (index .Vars "exclude")}}
{{$clientName := (index .Vars "client")}}
{{ $decorator := (printf "%s%s" (down $clientName) .Interface.Name) }}
{{ $Decorator := (printf "%s%s" $clientName .Interface.Name) }}
Expand All @@ -22,8 +23,8 @@ func New{{$Decorator}}(client {{.Interface.Type}}, timeout time.Duration) {{.Int
}

{{range $method := .Interface.Methods}}
{{if $method.AcceptsContext }}
func (c *{{$decorator}}) {{$method.Declaration}} {
func (c *{{$decorator}}) {{$method.Declaration}} {
{{- if and $method.AcceptsContext (not (has $method.Name $exclude)) }}
if ctx == nil {
ctx = context.Background()
}
Expand All @@ -32,7 +33,7 @@ func New{{$Decorator}}(client {{.Interface.Type}}, timeout time.Duration) {{.Int
ctx, cancelFunc = context.WithTimeout(ctx, c.timeout)
defer cancelFunc()
}
{{$method.Pass ("c.client.") }}
}
{{end}}
{{- end}}
{{$method.Pass ("c.client.") }}
}
{{end}}
64 changes: 0 additions & 64 deletions client/wrappers/timeout/history_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 24 additions & 1 deletion client/wrappers/timeout/history_generated_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"time"

"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
"go.uber.org/yarpc"

"github.com/uber/cadence/client/history"
Expand All @@ -53,7 +54,7 @@ func Test_historyClient_CloseShard(t *testing.T) {
{
name: "nil context success",
fields: fields{
timeout: time.Millisecond * 100,
timeout: time.Millisecond * 150,
},
args: args{
ctx: nil,
Expand Down Expand Up @@ -107,3 +108,25 @@ func Test_historyClient_CloseShard(t *testing.T) {
})
}
}

func Test_historyClient_GetReplicationMessages(t *testing.T) {
t.Run("no timeout", func(t *testing.T) {
m := history.NewMockClient(gomock.NewController(t))
m.EXPECT().GetReplicationMessages(gomock.Any(), gomock.Any()).DoAndReturn(func(ctx context.Context, r *types.GetReplicationMessagesRequest, opt ...yarpc.CallOption) (*types.GetReplicationMessagesResponse, error) {
for {
select {
case <-ctx.Done():
return nil, ctx.Err()
case <-time.After(time.Millisecond * 100):
return &types.GetReplicationMessagesResponse{}, nil
}
}
})
c := historyClient{
client: m,
timeout: time.Millisecond * 10,
}
_, err := c.GetReplicationMessages(context.Background(), &types.GetReplicationMessagesRequest{})
assert.NoError(t, err)
})
}

0 comments on commit e198ca3

Please sign in to comment.