-
Notifications
You must be signed in to change notification settings - Fork 17
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
fix(go): explicit usage of browse helpers #3417
Conversation
✔️ Code generated!
📊 Benchmark resultsBenchmarks performed on the method using a mock server, the results might not reflect the real-world performance.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great catch!
maxRetries: -1, | ||
timeout: func(count int) time.Duration { | ||
return 0 * time.Millisecond | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this expected to change defaults?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes it shouldn't wait by default, the function calling this should provide a default.
This was causing the browseObjects helper to wait 1s between each call for no reason
@@ -124,18 +111,18 @@ func (c *APIClient) WaitForTask( | |||
// provide a defalut timeout function | |||
opts = append([]IterableOption{WithTimeout(func(count int) time.Duration { | |||
return time.Duration(min(200*count, 5000)) * time.Millisecond | |||
})}, opts...) | |||
}), WithMaxRetries(50)}, opts...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah ok it's here now
🧭 What and Why
Don't silence the error on
CreateIterable
, add some doc, and make the usage more explicit by only returningerror
and force to useWithAggragator
.