Go Windows Azure Client Library

Merge lp://staging/~jtv/gwacl/require-storage-location into lp://staging/gwacl

  1. require-storage-location
  2. Merge into trunk
Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: 218
Merged at revision: 215
Proposed branch: lp://staging/~jtv/gwacl/require-storage-location
Merge into: lp://staging/gwacl
Diff against target: 334 lines (+73/-48)
5 files modified
helpers_http_test.go (+3/-2)
storage_base.go (+5/-14)
storage_base_test.go (+44/-16)
storage_test.go (+20/-16)
testing_test.go (+1/-0)
To merge this branch: bzr merge lp://staging/~jtv/gwacl/require-storage-location
Reviewer Review Type Date Requested Status
Julian Edwards (community) Approve
Review via email: mp+178721@code.staging.launchpad.net

Commit message

Make StorageContext.AzureEndpoint required.

Description of the change

Unfortunately, making this required is a runtime-incompatible API change. That is, client code that fails to provide it will still compile but it will panic at run time.

There was a lot of collateral damage, as expected. In particular, two storage tests relied on some arbitrary hard-coded names (not even mentioned in the tests themselves!) being identical between various helper functions. Luckily it wasn't hard to clean this up a bit. The tests themselves are now the only source of those arbitrary strings.

The test-helper factory for storage contexts sets an "unguessable" AzureEndpoint by default, so that we don't accidentally rely on such implicit connections here. Tests that need a known string, have to set one. As per the normal policy I used example.com, except of course in the tests for the functions that actually generate those strings. Those are already acceptance-tested against the real-world URLs, although it probably won't show up in the diff here.

Jeroen

To post a comment you must log in.
Revision history for this message
Julian Edwards (julian-edwards) wrote :

Looks great! Good testing as always from you.

8 - Account: MakeRandomString(10),
9 - Key: base64.StdEncoding.EncodeToString(MakeRandomByteSlice(10)),
10 + Account: MakeRandomString(10),
11 + Key: base64.StdEncoding.EncodeToString(MakeRandomByteSlice(10)),
12 + AzureEndpoint: APIEndpoint("http://" + MakeRandomString(5) + ".example.com/"),

I'd like to just take a moment to bitch at gofmt for changing lines completely unrelated to the diff at hand. >:(

44 + if context.AzureEndpoint == APIEndpoint("") {
45 + panic(errors.New("no Azure blob storage endpoint specified"))

Can you mention the parameter by name please, it will save a confused developer from hunting down this line of source.

review: Approve
lp://staging/~jtv/gwacl/require-storage-location updated
218. By Jeroen T. Vermeulen

Name AzureEndpoint in panic text.

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Collateral damage is a recurring problem in Go.

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