-
Notifications
You must be signed in to change notification settings - Fork 110
0.12.3 backports #433
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
0.12.3 backports #433
Conversation
* ci: Pin tracing-attributes to fix CI * Pin tracing-core
See apache#339. The old name is kept as a deprecated alias for now.
This used to work once but got broken during refactoring. Added a test to catch regressions. See apache#48 where -- I think -- we decided that instead of printing entire error chains in `Display` the API users shall walk the error chain. This is especially relevant for error type that we do NOT control, like upstream `request` types.
This code is what HTTP uses to indicate the client has sent too many requests in a given amount of time. It may sound counter-intuitive to retry in this situation, but that's why we use an exponential backoff mechanism. It gives the server the opportunity to recover, without failing the requests immediately. The retry mechanism already works in object stores like S3 because they return a server error. But without this change, we are not handling GCS properly. GCS returns a client error `429 Too Many Requests` instead. This change enables retries on this response too. A more advanced retry mechanism would use the optional response header `Retry-After`, but that is beyond the scope of this PR. Closes: apache#309
|
|
||
| // See <https://github.com/apache/arrow-rs-object-store/issues/339>. | ||
| #[doc(hidden)] | ||
| #[deprecated(note = "Use PutMultipartOptions", since = "0.13.0")] |
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.
Maybe we should use a different version here?
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.
Since this needs to be fixed on main too, I'm gonna fix that in a release notes PR which needs to land on main anyways.
|
Hello! Please excuse me interrupting the release flow, but do you think it could be possible to fit in this very small MR (not yet merged nor approved nor reviewed)? #431 |
No, not merged on main, hence no cherry-picking. |
|
I've manually merged the PR using git because GitHub insisted on squashing the commits -- which would have been a mess. |
See #428.
Content