Merge lp://staging/~bigkevmcd/goose/fix-for-swift-headers into lp://staging/goose

Proposed by Kevin McDermott
Status: Merged
Approved by: John A Meinel
Approved revision: 122
Merged at revision: 123
Proposed branch: lp://staging/~bigkevmcd/goose/fix-for-swift-headers
Merge into: lp://staging/goose
Diff against target: 306 lines (+114/-21)
6 files modified
http/client.go (+14/-11)
http/client_test.go (+3/-0)
swift/live_test.go (+17/-2)
swift/swift.go (+8/-8)
testservices/swiftservice/service_http.go (+22/-0)
testservices/swiftservice/service_http_test.go (+50/-0)
To merge this branch: bzr merge lp://staging/~bigkevmcd/goose/fix-for-swift-headers
Reviewer Review Type Date Requested Status
John A Meinel Approve
Review via email: mp+218073@code.staging.launchpad.net

Commit message

This is a fix for this, basically, this code...

http/client.go

 func (c *Client) sendRequest(method, URL string, reqReader io.Reader, length int, headers http.Header, expectedStatus []int, logger *log.Logger) (*http.Response, error) {

originally, only returned the body of the response, which meant there was no scope for getting the headers.

Coupled with this code from swift/swift.go

-func (c *Client) HeadObject(containerName, objectName string) (headers http.Header, err error)

the fact that it's creating headers here, masks the fact that they're not originating in the request.

I implemented HEAD in the fake server, mainly because it's the quickest way to test the fix, we don't necessarily need HEAD, but we wanted to proxy the headers returned by the Swift request to downstream clients (specifically ETag / cache-control headers).

Description of the change

This is a fix for this, basically, this code...

http/client.go

 func (c *Client) sendRequest(method, URL string, reqReader io.Reader, length int, headers http.Header, expectedStatus []int, logger *log.Logger) (*http.Response, error) {

originally, only returned the body of the response, which meant there was no scope for getting the headers.

Coupled with this code from swift/swift.go

-func (c *Client) HeadObject(containerName, objectName string) (headers http.Header, err error)

the fact that it's creating headers here, masks the fact that they're not originating in the request.

I implemented HEAD in the fake server, mainly because it's the quickest way to test the fix, we don't necessarily need HEAD, but we wanted to proxy the headers returned by the Swift request to downstream clients (specifically ETag / cache-control headers).

To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :

LGTM. Thanks for doing this. This does end up changing the public API, but I can understand that you need some way to get the headers , and we don't currently expose it.

We could return the raw http.Response object, but ATM I think this provides the best balance of preserving some flexibility in the response and giving enough information for users.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches