-
Notifications
You must be signed in to change notification settings - Fork 430
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: retryable config enablement through storage_options #3129
feat: retryable config enablement through storage_options #3129
Conversation
eb8fb9b
to
96179b0
Compare
Signed-off-by: Ion Koutsouris <[email protected]>
96179b0
to
4085c3d
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3129 +/- ##
==========================================
- Coverage 72.28% 72.23% -0.05%
==========================================
Files 134 134
Lines 42973 43059 +86
Branches 42973 43059 +86
==========================================
+ Hits 31062 31104 +42
- Misses 9923 9956 +33
- Partials 1988 1999 +11 ☔ View full report in Codecov by Sentry. |
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.
Just some small nits to look at.
let inner = builder | ||
.with_retry(self.parse_retry_config(&options)?) | ||
.build()?; | ||
|
||
let store = limit_store_handler(url_prefix_handler(inner, prefix.clone()), options); |
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.
Not related to this PR, but why do we wrap this in the limiting store again?
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.
Its due to this PR: #2458
I will put a small PR to actually document this storage 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.
@roeap I've added docs in this PR, and also included the the other undocumented options :)
Signed-off-by: Ion Koutsouris <[email protected]>
8564ac9
to
4c46287
Compare
Signed-off-by: Ion Koutsouris <[email protected]>
b94227d
to
a2639e9
Compare
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.
Thank you for documenting some of these magic hidden variables a bit better 😄
I see you're really loving traits lately 😆
Description