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: allow retry 0 directives #2479

Merged
merged 1 commit into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions backend/schema/metadataretry.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,17 +109,21 @@ func (m *MetadataRetry) RetryParams() (RetryParams, error) {

// min backoff
if m.MinBackoff == "" {
return RetryParams{}, fmt.Errorf("retry must have a minimum backoff")
}
minBackoff, err := parseRetryDuration(m.MinBackoff)
if err != nil {
return RetryParams{}, fmt.Errorf("could not parse min backoff duration: %w", err)
if params.Count != 0 {
return RetryParams{}, fmt.Errorf("retry must have a minimum backoff")
}
params.MinBackoff = MinBackoffLimit
} else {
minBackoff, err := parseRetryDuration(m.MinBackoff)
if err != nil {
return RetryParams{}, fmt.Errorf("could not parse min backoff duration: %w", err)
}
params.MinBackoff = minBackoff
}
params.MinBackoff = minBackoff

// max backoff
if m.MaxBackoff == "" {
params.MaxBackoff = max(minBackoff, DefaultMaxBackoff)
params.MaxBackoff = max(params.MinBackoff, DefaultMaxBackoff)
} else {
maxBackoff, err := parseRetryDuration(m.MaxBackoff)
if err != nil {
Expand Down
125 changes: 121 additions & 4 deletions backend/schema/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ func TestParserRoundTrip(t *testing.T) {

//nolint:maintidx
func TestParsing(t *testing.T) {
zero := 0
ten := 10
tests := []struct {
name string
Expand Down Expand Up @@ -428,11 +429,23 @@ func TestParsing(t *testing.T) {
+retry 1h1m5s
verb C(Empty) Unit
+retry 0h0m5s 1h0m0s

fsm FSM {
verb D(Empty) Unit
+retry 0
verb E(Empty) Unit
+retry 0 1s catch test.catch
verb F(Empty) Unit
+retry 0 catch test.catch
verb catch(builtin.CatchRequest<Any>) Unit

fsm FSM
+ retry 10 1s 10s
{
start test.A
transition test.A to test.B
transition test.A to test.C
transition test.B to test.C
transition test.C to test.D
transition test.D to test.E
transition test.E to test.F
}
}
`,
Expand All @@ -442,6 +455,13 @@ func TestParsing(t *testing.T) {
Decls: []Decl{
&FSM{
Name: "FSM",
Metadata: []Metadata{
&MetadataRetry{
Count: &ten,
MinBackoff: "1s",
MaxBackoff: "10s",
},
},
Start: []*Ref{
{
Module: "test",
Expand All @@ -462,13 +482,43 @@ func TestParsing(t *testing.T) {
{
From: &Ref{
Module: "test",
Name: "A",
Name: "B",
},
To: &Ref{
Module: "test",
Name: "C",
},
},
{
From: &Ref{
Module: "test",
Name: "C",
},
To: &Ref{
Module: "test",
Name: "D",
},
},
{
From: &Ref{
Module: "test",
Name: "D",
},
To: &Ref{
Module: "test",
Name: "E",
},
},
{
From: &Ref{
Module: "test",
Name: "E",
},
To: &Ref{
Module: "test",
Name: "F",
},
},
},
},
&Verb{
Expand Down Expand Up @@ -525,6 +575,73 @@ func TestParsing(t *testing.T) {
},
},
},
&Verb{
Name: "D",
Request: &Ref{
Module: "builtin",
Name: "Empty",
},
Response: &Unit{
Unit: true,
},
Metadata: []Metadata{
&MetadataRetry{
Count: &zero,
},
},
},
&Verb{
Name: "E",
Request: &Ref{
Module: "builtin",
Name: "Empty",
},
Response: &Unit{
Unit: true,
},
Metadata: []Metadata{
&MetadataRetry{
Count: &zero,
MinBackoff: "1s",
Catch: &Ref{
Module: "test",
Name: "catch",
},
},
},
},
&Verb{
Name: "F",
Request: &Ref{
Module: "builtin",
Name: "Empty",
},
Response: &Unit{
Unit: true,
},
Metadata: []Metadata{
&MetadataRetry{
Count: &zero,
Catch: &Ref{
Module: "test",
Name: "catch",
},
},
},
},
&Verb{
Name: "catch",
Request: &Ref{
Module: "builtin",
Name: "CatchRequest",
TypeParameters: []Type{
&Any{},
},
},
Response: &Unit{
Unit: true,
},
},
},
}},
},
Expand Down
7 changes: 5 additions & 2 deletions backend/schema/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -877,8 +877,8 @@ func validateVerbSubscriptions(module *Module, v *Verb, md *MetadataSubscriber,

func validateRetries(module *Module, retry *MetadataRetry, requestType optional.Option[Type], scopes Scopes, schema optional.Option[*Schema]) (merr []error) {
// Validate count
if retry.Count != nil && *retry.Count <= 0 {
merr = append(merr, errorf(retry, "retry count must be at least 1"))
if retry.Count != nil && *retry.Count < 0 {
merr = append(merr, errorf(retry, "retry count can not be negative"))
}

// Validate parsing of durations
Expand All @@ -893,6 +893,9 @@ func validateRetries(module *Module, retry *MetadataRetry, requestType optional.

// validate catch
if retry.Catch == nil {
if retryParams.Count == 0 && retry.MinBackoff != "" {
merr = append(merr, errorf(retry, "can not define a backoff duration when retry count is 0 and no catch is declared"))
}
return
}
req, ok := requestType.Get()
Expand Down
3 changes: 1 addition & 2 deletions backend/schema/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,9 +380,8 @@ func TestValidate(t *testing.T) {
`19:7-7: could not parse min backoff duration: could not parse retry duration: duration has unknown unit "mins" - use 'd', 'h', 'm' or 's', eg '1d' or '30s'`,
`21:7-7: max backoff duration (1s) needs to be at least as long as initial backoff (1m)`,
`23:7-7: could not parse min backoff duration: retry backoff can not be larger than 1d`,
`25:7-7: retry count must be at least 1`,
`25:7-7: can not define a backoff duration when retry count is 0 and no catch is declared`,
`30:7-7: catch can only be defined on verbs`,
`30:7-7: retry count must be at least 1`,
`4:7-7: could not parse min backoff duration: could not parse retry duration: duration has unit "m" out of order - units need to be ordered from largest to smallest - eg '1d3h2m'`,
`6:7-7: could not parse min backoff duration: could not parse retry duration: duration has unit "d" out of order - units need to be ordered from largest to smallest - eg '1d3h2m'`,
`8:7-7: could not parse min backoff duration: retry must have a minimum backoff of 1s`,
Expand Down
Loading