Respect common HTTP headers when uploading Azure Storage blob via stream #1553
Conversation
| @@ -929,6 +932,31 @@ def delete_object(self, obj): | |||
|
|
|||
| return False | |||
|
|
|||
| def _fix_headers(self, headers): | |||
There was a problem hiding this comment.
Can you please add some unit tests for this method / functionality?
There was a problem hiding this comment.
Fitting this into the mock request/response framework was proving challenging so I added an isolated test for the new normalization functionality in 98cd383. I also added an integration test for this scenario in c-w/libcloud-tests@e305b9d.
There was a problem hiding this comment.
Investigating why the new e2e test fails for Azurite v3 only (see test run).
There was a problem hiding this comment.
Yeah, direct unit tests for that method are fine :)
Maybe something to do with auth signature? Since, IIRC, we use header values when generating the signature.
There was a problem hiding this comment.
On that note - I wonder if we could some merge some of that end to end tests in that repo (if it makes sense and how we could do that).
There was a problem hiding this comment.
I believe the test failure may be linked to an issue in AzuriteV3, see Azure/Azurite#629 (comment). Given that the test passes for all other storage targets, I disabled the test for AzuriteV3 in c-w/libcloud-tests@b2be083.
…eam (apache#1553) * Respect headers uploading Azure blob via stream * Add tests for _fix_headers
…eam (apache#1553) * Respect headers uploading Azure blob via stream * Add tests for _fix_headers
Respect common HTTP headers when uploading Azure Storage blob via stream
Description
Resolves #1550
As per the documentation, when uploading an object via stream to Azure Storage, common HTTP headers (e.g.
Content-Encoding) must be provided via an Azure-specific name (e.g.x-ms-blob-content-encoding). The driver was previously ignoring user-provided headers when uploading objects via stream and wasn't adapting the header names to their Azure-specific equivalent, which are both issues fixed by this pull request.Status
Checklist (tick everything that applies)