fix: include existing headers in prepare request #309
Conversation
product-auto-label
bot
added
the
api: storage
There was a problem hiding this comment.
As talked about in a Chat thread, it appears this reverts fd47338
From what I can tell, that was done purely as an optimization, so I don't see this causing any problems.
I do recommend running this patch against the GCS system tests just to be sure.
tests/unit/test__upload.py
Outdated
| @@ -621,12 +621,11 @@ def test__prepare_request_success_with_headers(self): | |||
| new_headers = self._prepare_request_helper(headers) | |||
| assert new_headers is not headers | |||
| expected_headers = { | |||
| "cannot": "touch this", | |||
There was a problem hiding this comment.
This test is very subtle and relies on
to pass in new headers when callingself._upload_in_flight(data, headers=headers, checksum=checksum). IIUC, the test is to prevent any regressions from future changes (super not clear by the name of the test).
What I don't understand is what changed when **self._headers is added so it can be tested. @Breathtender could add a test that validates the behavior change intended by adding **self._headers?
There was a problem hiding this comment.
@Breathtender i spoke with @andrewsg and he clarified what was going on. Apologies, I misread this code block completely. "cannot": "touch this" is a bit distracting and would recommend changing it to: "keep": "must be kept".
There was a problem hiding this comment.
Oh that's a good change lol
tests/unit/test__upload.py
Outdated
| @@ -621,12 +621,11 @@ def test__prepare_request_success_with_headers(self): | |||
| new_headers = self._prepare_request_helper(headers) | |||
| assert new_headers is not headers | |||
| expected_headers = { | |||
| "cannot": "touch this", | |||
There was a problem hiding this comment.
@Breathtender i spoke with @andrewsg and he clarified what was going on. Apologies, I misread this code block completely. "cannot": "touch this" is a bit distracting and would recommend changing it to: "keep": "must be kept".
…media-python into useragent
dandhlee
added
the
owlbot:run
gcf-owl-bot
bot
removed
the
owlbot:run
This is a needed change to allow us to properly set user agent from the python-storage library, for multi-chunked uploads.