You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "universalmind303 (via GitHub)" <gi...@apache.org> on 2023/09/19 23:35:03 UTC
[GitHub] [arrow-rs] universalmind303 opened a new pull request, #4841: fix: object store http range requests
universalmind303 opened a new pull request, #4841:
URL: https://github.com/apache/arrow-rs/pull/4841
# Which issue does this PR close?
Closes #4839
# Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
# What changes are included in this PR?
If a range was requested and the server returned a 200 instead of a 206, we error out as it means that the server didn't fulfill the request.
# Are there any user-facing changes?
this could cause some backwards incompatibility with previously working queries no longer working as we are now a bit stricter about what we are expecting.
Some examples that would no longer work
- a server _could_ satisfy the range request, but due to misconfiguration is still returning a `200`.
- the server never satisfied the range request & we fetched the entire data multiple times.
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!---
If there are any breaking changes to public APIs, please add the `breaking change` label.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] tustvold merged pull request #4841: Error if Remote Ignores HTTP Range Header
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold merged PR #4841:
URL: https://github.com/apache/arrow-rs/pull/4841
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] tustvold commented on a diff in pull request #4841: fix: object store http range requests
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #4841:
URL: https://github.com/apache/arrow-rs/pull/4841#discussion_r1330998867
##########
object_store/src/http/client.rs:
##########
@@ -252,7 +256,20 @@ impl Client {
}
}
_ => Error::Request { source }.into(),
- })
+ })?;
+
+ // We expect a 206 Partial Content response if a range was requested
+ // a 200 OK response would indicate the server did not fulfill the request
+ if has_range && res.status() == StatusCode::OK {
+ return Err(crate::Error::Generic {
Review Comment:
I think this should possibly be the NotSupported variant
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-rs] tustvold commented on a diff in pull request #4841: Error if Remote Ignores HTTP Range Header
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #4841:
URL: https://github.com/apache/arrow-rs/pull/4841#discussion_r1331670937
##########
object_store/src/http/client.rs:
##########
@@ -252,7 +256,19 @@ impl Client {
}
}
_ => Error::Request { source }.into(),
- })
+ })?;
+
+ // We expect a 206 Partial Content response if a range was requested
+ // a 200 OK response would indicate the server did not fulfill the request
+ if has_range && res.status() == StatusCode::OK {
Review Comment:
```suggestion
if has_range && res.status() != StatusCode::PARTIAL_CONTENT {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org