-
Notifications
You must be signed in to change notification settings - Fork 36
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: Add "retry" networkservice.Client wrapper for nscs #1129
feat: Add "retry" networkservice.Client wrapper for nscs #1129
Conversation
Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>
Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>
c104398
to
49fa488
Compare
Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com>
type Option func(*retryClient) | ||
|
||
// WithSettings sets retry policy settings for the retry.Client instance. | ||
func WithSettings(s Settings) Option { |
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.
Why not do this as standard options like
retry.WithInterval
retry.WithTryTimeout
instead of 'Settings'
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.
Thanks, fixed.
|
||
// Settings represents retry policy settings. | ||
type Settings struct { | ||
Timeout time.Duration |
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.
Why have timeout here instead of simply using the ctx passes into the RPC calls as normal?
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.
Thanks, fixed.
for ctx.Err() == nil { | ||
requestCtx, cancel := c.WithTimeout(ctx, r.TryTimeout) | ||
resp, err := r.client.Request(requestCtx, request, opts...) | ||
cancel() |
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.
Shouldn't this be
cancel() | |
defer cancel() |
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.
Thanks, fixed.
Signed-off-by: denis-tingaikin <denis.tingajkin@xored.com>
…k@main PR link: networkservicemesh/sdk#1129 Commit: 0eabc52 Author: Denis Tingaikin Date: 2021-11-02 04:35:19 +0300 Message: - feat: Add "retry" networkservice.Client wrapper for nscs (#1129) * add re-try networkservice.Client wrapper Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * fix linter issue Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * update default values after manul testing Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * apply review comments Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
…k@main PR link: networkservicemesh/sdk#1129 Commit: 0eabc52 Author: Denis Tingaikin Date: 2021-11-02 04:35:19 +0300 Message: - feat: Add "retry" networkservice.Client wrapper for nscs (#1129) * add re-try networkservice.Client wrapper Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * fix linter issue Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * update default values after manul testing Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * apply review comments Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
…k@main PR link: networkservicemesh/sdk#1129 Commit: 0eabc52 Author: Denis Tingaikin Date: 2021-11-02 04:35:19 +0300 Message: - feat: Add "retry" networkservice.Client wrapper for nscs (#1129) * add re-try networkservice.Client wrapper Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * fix linter issue Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * update default values after manul testing Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * apply review comments Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
…k@main PR link: networkservicemesh/sdk#1129 Commit: 0eabc52 Author: Denis Tingaikin Date: 2021-11-02 04:35:19 +0300 Message: - feat: Add "retry" networkservice.Client wrapper for nscs (#1129) * add re-try networkservice.Client wrapper Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * fix linter issue Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * update default values after manul testing Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * apply review comments Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
…k@main PR link: networkservicemesh/sdk#1129 Commit: 0eabc52 Author: Denis Tingaikin Date: 2021-11-02 04:35:19 +0300 Message: - feat: Add "retry" networkservice.Client wrapper for nscs (#1129) * add re-try networkservice.Client wrapper Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * fix linter issue Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * update default values after manul testing Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * apply review comments Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
…k@main PR link: networkservicemesh/sdk#1129 Commit: 0eabc52 Author: Denis Tingaikin Date: 2021-11-02 04:35:19 +0300 Message: - feat: Add "retry" networkservice.Client wrapper for nscs (#1129) * add re-try networkservice.Client wrapper Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * fix linter issue Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * update default values after manul testing Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * apply review comments Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
…k@main PR link: networkservicemesh/sdk#1129 Commit: 0eabc52 Author: Denis Tingaikin Date: 2021-11-02 04:35:19 +0300 Message: - feat: Add "retry" networkservice.Client wrapper for nscs (#1129) * add re-try networkservice.Client wrapper Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * fix linter issue Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * update default values after manul testing Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * apply review comments Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
…k@main PR link: networkservicemesh/sdk#1129 Commit: 0eabc52 Author: Denis Tingaikin Date: 2021-11-02 04:35:19 +0300 Message: - feat: Add "retry" networkservice.Client wrapper for nscs (#1129) * add re-try networkservice.Client wrapper Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * fix linter issue Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * update default values after manul testing Signed-off-by: Denis Tingaikin <denis.tingajkin@xored.com> * apply review comments Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
Signed-off-by: Denis Tingaikin denis.tingajkin@xored.com
Issue link
#1123
How Has This Been Tested?
Types of changes