Merge lp:~julian-edwards/gwacl/pageblob into lp:gwacl
Proposed by
Julian Edwards
Status: | Merged |
---|---|
Approved by: | Julian Edwards |
Approved revision: | 114 |
Merged at revision: | 111 |
Proposed branch: | lp:~julian-edwards/gwacl/pageblob |
Merge into: | lp:gwacl |
Diff against target: |
147 lines (+71/-10) 2 files modified
storage_base.go (+31/-5) storage_base_test.go (+40/-5) |
To merge this branch: | bzr merge lp:~julian-edwards/gwacl/pageblob |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jeroen T. Vermeulen (community) | Approve | ||
Review via email: mp+170009@code.launchpad.net |
Commit message
Fix PutBlob by making sure it can supply x-ms-blob-type in the request.
Description of the change
The existing PutBlob implementation was half-finished. I think we originally started it with good intentions and then realised it wasn't needed.
This branch finishes the work by enabling the required x-ms-blob-type header. Some changes were needed in performRequest to support this.
We need to use PutBlob to initialise page blobs for the proposed user data solution.
To post a comment you must log in.
The capitalization of PutBlobRequest looks suspicious: the type is exported but its attributes are not. AFAICS its only use is in PutBlob, but how is anybody outside the module going to be able to create a PutBlobRequest to pass to PutBlob if they can't address its fields?
Is there any value to passing the address of a PutBlobRequest into PutBlob, as opposed to just giving it the struct itself? It just seems like extra weird syntax to live with: &PutBlockReques t{...} instead of just PutBlockRequest {...}. Then again, it's probably consistent with what we did in other places...
By the way, blob_type in Go style becomes blobType, and extra_headers becomes extraHeaders. Not pleasant, especially when it goes against vocal emphasis, but we'd better get used to it.
Once inside PutBlob, you return an error when an invalid blob type is selected. But this isn't variable data, so I'd say this was indicative of a static programming mistake. Might as well panic to teach them a lesson. And if that seems unfair, well, document the thing so they get a chance to do better!
The comment on TestPutBlob, about PutBlob returning Created on success, no longer belongs on top... It's now one of many things that the test looks at.
On TestPutPageBlob, the documenting comment incongruously starts with "Ensure that." Might as well drop that and just state the behaviour that the test verifies as fact.