Handle empty S3 payloads (#4514)#4518
Conversation
alamb
left a comment
There was a problem hiding this comment.
The code looks good, but I am not sure about the test
|
|
||
| delete_fixtures(storage).await; | ||
|
|
||
| let path = Path::from("empty"); |
There was a problem hiding this comment.
This test appears to pass even without the code changes in this PR. See #4519 for example
|
You need to test against real S3, the emulator doesn't seem to exhibit the same issue |
I will try |
|
@tustvold can you give me the command you used run the tests against a real S3 bucket (obviously I will supply my own AWS credentials) I got this far (but then I stumbled on what the "metadata endpoint" should be |
|
Just omit the endpoint, note also that the credential names should be prefixed by OBJECT_STORE |
alamb
left a comment
There was a problem hiding this comment.
Ok, I ran the new test against an actual S3 bucket and without the code changes in this PR it fails like this:
---- aws::tests::s3_test stdout ----
thread 'aws::tests::s3_test' panicked at 'called `Result::unwrap()` on an `Err` value: Generic { store: "S3", source: Error { retries: 0, message: "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<Error><Code>MissingContentLength</Code><Message>You must provide the Content-Length HTTP header.</Message><RequestId>....</RequestId><HostId>....</HostId></Error>", source: Some(reqwest::Error { kind: Status(411), url: Url { scheme: "https", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("s3.us-east-1.amazonaws.com")), port: None, path: "/alamb-test-bucket/empty", query: None, fragment: None } }), status: Some(411) } }', src/lib.rs:1263:48
With this PR it passes
Nice work @tustvold -- thank you
Which issue does this PR close?
Closes apache/arrow-rs-object-store#155
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?