Merge lp:~allenap/gwacl/chunky-blobs-operation into lp:gwacl

[Gavin Panella]

New Storage API meta-operation UploadBlockBlob that uses PutBlock and PutBlockList to upload larger files in chunks.

[Julian Edwards (julian-edwards)]

61 === added file 'storage2.go'

Can you not think of a better name for this? In fact I'm not even sure we need another file. If you insist, we could have a new file called storage_blob.go and move all the blob stuff into it.

[Julian Edwards (julian-edwards)]

[sorry. hit the button too early]

91 + if err == io.EOF {
92 + break
93 + }
94 + if err != nil {
95 + return err
96 + }

I looked at this and thought, why not a switch. But then the break would break the wrong thing... gah :/

98 + blockID := strconv.FormatInt(blockNum, 36)

The documentation says that all block IDs should be the same length, and suggests base64 encoding this ID. Additionally the docs say "the pre-encoded string must be 64 bytes or less". It might be worth enforcing/checking that here.

99 + log.Printf("Uploading block %d (size=%d, id=%s).\n",
109 + log.Printf("Committing %d blocks.\n", len(blockList.Items))

I don't think you should be logging here :)

You don't have to do it here but one of the advantages of block uploads is that we can do them in parallel. As a side project you could add go routines to do that here, up to a configured maximum in parallel. It'll speed up the uploads considerably.

167 + Body: ioutil.NopCloser(bytes.NewReader(nil)),

We've used this NopCloser thing in a few places now, do you think we should turn it into a test helper?

160 +func (suite *testUploadBlockBlob) TestNoHeaders(c *C) {

Did you copy this and forget to change the function name?

172 + Status: fmt.Sprintf("%d", http.StatusOK),
173 + StatusCode: http.StatusCreated,

Discrepancy between statuses here, is that intentional? If so, please comment why.

I don't know if it's how Go just is but I found it quite hard to discern the test intentions in this large test. I think we should break it up for readability, and create some re-usable test helpers at the same time.

If we had something along the lines of this, it would more clearly show test intent:
[in pseudo code]
    insertFakeResponse(status=..., body=)
    makeRandomData()
    uploadBlockBlob()
    assertPutBlockSent(url, data)
    assertBlockListSent(url, data)

This way we've abstracted out the boilerplate, but the test data is all contained here in the same function so it's easy to see input and expected output, and the test intention.

review: Needs Fixing

[lp:~allenap/gwacl/chunky-blobs-operation]

100. By Gavin Panella on 2013-04-19

Create Empty, an io.ReadCloser counterpart to ioutil.Discard.

101. By Gavin Panella on 2013-04-19

Fix copy-n-paste error.

102. By Gavin Panella on 2013-04-19

Fix typo.

[Gavin Panella (allenap)]

> [sorry. hit the button too early]
>
> 91      +        if err == io.EOF {
> 92      +            break
> 93      +        }
> 94      +        if err != nil {
> 95      +            return err
> 96      +        }
>
> I looked at this and thought, why not a switch.  But then the break would
> break the wrong thing... gah :/

Yeah, that was the same problem I encountered. I could use a label,
but they're just as ugly.

>
> 98      +        blockID := strconv.FormatInt(blockNum, 36)
>
> The documentation says that all block IDs should be the same length, and
> suggests base64 encoding this ID. Additionally the docs say "the pre-encoded
> string must be 64 bytes or less".  It might be worth enforcing/checking that
> here.

By my calculations we won't have a problem here until blockNum is a
little over 4e99. There's belt and braces, then there's girdle,
suspenders and nappies. I think it'll be fine :)

>
> 99      +        log.Printf("Uploading block %d (size=%d, id=%s).\n",
> 109     +    log.Printf("Committing %d blocks.\n", len(blockList.Items))
>
> I don't think you should be logging here :)

I think I should be logging here, but for Go's pretend log package,
which is not much more than a wrapper around print right now.
Crucially there's no way to suppress these messages. If you object
strongly I'll remove them, but they are useful when uploading bigger
files.

>
> You don't have to do it here but one of the advantages of block uploads is
> that we can do them in parallel.  As a side project you could add go routines
> to do that here, up to a configured maximum in parallel.  It'll speed up the
> uploads considerably.

Will it? If network utilisation is near 100% then doing more
concurrently won't speed it up. I guess it would mean that more TCP
slow-starts could happen together, instead of each time there's a
connection. Beyond that I'm not sure. I want to get my teeth into some
Go-style concurrency, but I'm not sure this warrants it.

>
> 167     +        Body:       ioutil.NopCloser(bytes.NewReader(nil)),
>
> We've used this NopCloser thing in a few places now, do you think we should
> turn it into a test helper?

Done: I've created Empty in test_helpers.go, which is a io.ReadCloser
counterpart to ioutil.Discard.

>
> 160     +func (suite *testUploadBlockBlob) TestNoHeaders(c *C) {
>
> Did you copy this and forget to change the function name?

Oops, yes.

>
> 172     +        Status:     fmt.Sprintf("%d", http.StatusOK),
> 173     +        StatusCode: http.StatusCreated,
>
> Discrepancy between statuses here, is that intentional?  If so, please comment
> why.

It was a mistake. Fixed.

>
> I don't know if it's how Go just is but I found it quite hard to discern the
> test intentions in this large test.  I think we should break it up for
> readability, and create some re-usable test helpers at the same time.
>
> If we had something along the lines of this, it would more clearly show test
> intent:
> [in pseudo code]
>     insertFakeResponse(status=..., body=)
>     makeRandomData()
>     uploadBlockBlob()
>     assertPutBlockSent(url, data)
>     assertBlockListSent(url, data)
>
> This way we've abstracted out the boilerplate, but the tes...

[Gavin Panella (allenap)]

> 61      === added file 'storage2.go'
>
> Can you not think of a better name for this?  In fact I'm not even sure we
> need another file.  If you insist, we could have a new file called
> storage_blob.go and move all the blob stuff into it.

I tried first to refactor the storage stuff into a subpackage, but
that turned into a nightmare, and I backed out. The other file was
getting a bit long, and I wanted to put these higher-level operations
somewhere distinct from the lower-level API building blocks. As such,
I don't think storage_blob.go is the right name. Perhaps renaming
storage.go to storage_base.go, and storage2.go to storage.go?

[lp:~allenap/gwacl/chunky-blobs-operation]

103. By Gavin Panella on 2013-04-19

Extract method makeFakeCreatedResponse().

104. By Gavin Panella on 2013-04-19

Extract methods uploadRandomBlob(), assertBlockSent(), and assertBlockListSent().

105. By Gavin Panella on 2013-04-19

Move makeFakeCreatedResponse() down.

106. By Gavin Panella on 2013-04-19

Additional test for a large file that actually needs chunking.

[Gavin Panella (allenap)]

I think this is ready for another look now. In addition to responding to your review comments, I've also added a second test for a larger file, one that needs to be chunked.

[Julian Edwards (julian-edwards)]

On 20/04/13 00:50, Gavin Panella wrote:
> Yeah, that was the same problem I encountered. I could use a label,
> but they're just as ugly.

I reckon this is why Go still has a goto ...

>>
>> 98 + blockID := strconv.FormatInt(blockNum, 36)
>>
>> The documentation says that all block IDs should be the same length, and
>> suggests base64 encoding this ID. Additionally the docs say "the pre-encoded
>> string must be 64 bytes or less". It might be worth enforcing/checking that
>> here.
>
> By my calculations we won't have a problem here until blockNum is a
> little over 4e99. There's belt and braces, then there's girdle,
> suspenders and nappies. I think it'll be fine :)

I guess I am getting ahead of myself a bit, I was thinking of something
I know will happen at some point where people will want to supply their
own block IDs.

Does FormatInt guarantee the same length string for all numbers? That
does seem to be a hard requirement of the API.

>>
>> 99 + log.Printf("Uploading block %d (size=%d, id=%s).\n",
>> 109 + log.Printf("Committing %d blocks.\n", len(blockList.Items))
>>
>> I don't think you should be logging here :)
>
> I think I should be logging here, but for Go's pretend log package,
> which is not much more than a wrapper around print right now.
> Crucially there's no way to suppress these messages. If you object
> strongly I'll remove them, but they are useful when uploading bigger
> files.

I am strongly of the opinion that a library function should not
unconditionally log. At least change to a Debugf.

>> You don't have to do it here but one of the advantages of block uploads is
>> that we can do them in parallel. As a side project you could add go routines
>> to do that here, up to a configured maximum in parallel. It'll speed up the
>> uploads considerably.
>
> Will it?

Yes.

> If network utilisation is near 100% then doing more

You're not necessarily guaranteed 100% net utilisation from a single TCP
connection. Especially over international links. I can get much better
speeds here if I have more than one session at once, because handshaking
has such a high latency that the TCP window still fails to do anything
useful.

> concurrently won't speed it up. I guess it would mean that more TCP
> slow-starts could happen together, instead of each time there's a
> connection. Beyond that I'm not sure. I want to get my teeth into some
> Go-style concurrency, but I'm not sure this warrants it.

Trust me, it does warrant it. I do a lot of downloading from Australia
and I see real speed gains with multiple sessions.

Like I said, we don't need it right now in this branch but I think it
should be done. Besides, you want to play with goroutines, right? :)

>
>>
>> 167 + Body: ioutil.NopCloser(bytes.NewReader(nil)),
>>
>> We've used this NopCloser thing in a few places now, do you think we should
>> turn it into a test helper?
>
> Done: I've created Empty in test_helpers.go, which is a io.ReadCloser
> counterpart to ioutil.Discard.

Coolio, thanks! (I can't help but pronounce Body in the manner of Fat
Bastard from Austin Powers)

> I like this idea. Fwiw, I did pass this by The Mi...

[Julian Edwards (julian-edwards)]

On 20/04/13 00:55, Gavin Panella wrote:
>> 61 === added file 'storage2.go'
>>
>> Can you not think of a better name for this? In fact I'm not even sure we
>> need another file. If you insist, we could have a new file called
>> storage_blob.go and move all the blob stuff into it.
>
> I tried first to refactor the storage stuff into a subpackage, but
> that turned into a nightmare, and I backed out. The other file was
> getting a bit long, and I wanted to put these higher-level operations
> somewhere distinct from the lower-level API building blocks. As such,
> I don't think storage_blob.go is the right name. Perhaps renaming
> storage.go to storage_base.go, and storage2.go to storage.go?
>

On reflection I think it's early optimisation (if there is such a thing
for file organisation). Do you strongly object to just leaving it all
in the same file?

Otherwise your suggestion is fine.

[Julian Edwards (julian-edwards)]

+ // block ID is a base64 encoding of the string "0".
+ assertBlockSent(c, context, data, "MA==", transport.Exchanges[0])

You could use:
base64.StdEncoding.EncodeToString([]byte("0"))
like we do in the rest of the gwacl tests.

Everything else looks great, thanks.

review: Approve

[Gavin Panella (allenap)]

> Does FormatInt guarantee the same length string for all numbers?  That
> does seem to be a hard requirement of the API.

Ah, no, it doesn't. I haven't seen that requirement before... /me
investigates.

...
> I am strongly of the opinion that a library function should not
> unconditionally log.  At least change to a Debugf.

Yeah, I agree with that. It's a rubbish logging package. I'll do
something to make sure it's quiet.

>
> >> You don't have to do it here but one of the advantages of block uploads is
> >> that we can do them in parallel.  As a side project you could add go
> routines
> >> to do that here, up to a configured maximum in parallel.  It'll speed up
> the
> >> uploads considerably.
> >
> > Will it?
>
> Yes.
>
> > If network utilisation is near 100% then doing more
>
> You're not necessarily guaranteed 100% net utilisation from a single TCP
> connection.  Especially over international links.  I can get much better
> speeds here if I have more than one session at once, because handshaking
> has such a high latency that the TCP window still fails to do anything
> useful.
>
> > concurrently won't speed it up. I guess it would mean that more TCP
> > slow-starts could happen together, instead of each time there's a
> > connection. Beyond that I'm not sure. I want to get my teeth into some
> > Go-style concurrency, but I'm not sure this warrants it.
>
> Trust me, it does warrant it.  I do a lot of downloading from Australia
> and I see real speed gains with multiple sessions.
>
> Like I said, we don't need it right now in this branch but I think it
> should be done.  Besides, you want to play with goroutines, right? :)

Yes. It's about all the pleasure there's to be had from Go. Okay, I'll
try to add it when I'm stalling on something else.

[Gavin Panella (allenap)]

> On reflection I think it's early optimisation (if there is such a thing
> for file organisation). Do you strongly object to just leaving it all
> in the same file?

I do, if only because it's more work to split it out than it is to
merge it later if we decide we want it that way. It all appears in the
gwacl package, but I'd like to maintain a mental fence between the API
atoms and the things that are built with them.

[lp:~allenap/gwacl/chunky-blobs-operation]

107. By Gavin Panella on 2013-04-22

Add logging package to wrap the standard library's log package into something just about useful.

108. By Gavin Panella on 2013-04-22

Use the new logging stuff in storage2.go.

109. By Gavin Panella on 2013-04-22

Remove superflous semicolon.

110. By Gavin Panella on 2013-04-22

Encode the blockIDs in the test, instead of hard-coding the base64 versions.

111. By Gavin Panella on 2013-04-22

Add GPL header to logging.go.

112. By Gavin Panella on 2013-04-22

Make WARN the default log level.

113. By Gavin Panella on 2013-04-22

Set log.level from the environment.

114. By Gavin Panella on 2013-04-22

Renames.

115. By Gavin Panella on 2013-04-22

No need to define level's type.

116. By Gavin Panella on 2013-04-22

Add a rudimentary test for the logging stuff.

117. By Gavin Panella on 2013-04-22

Move logging stuff into subpackage.

118. By Gavin Panella on 2013-04-22

Use the logging package in x509dispatcher too.

=== modified file 'example/storage/run.go'
--- example/storage/run.go	2013-04-17 10:11:07 +0000
+++ example/storage/run.go	2013-04-22 16:24:27 +0000
@@ -10,15 +10,11 @@
 package main
 
 import (
-    "bytes"
     "flag"
     "fmt"
-    "io"
     "io/ioutil"
     "launchpad.net/gwacl"
-    "log"
     "os"
-    "strconv"
 )
 
 var account string
@@ -125,35 +121,11 @@
     }
     defer file.Close()
 
-    buffer := make([]byte, 1024*1024) // 1MB buffer
-    blockList := &gwacl.BlockList{}
-
-    // Upload the file in chunks.
-    for blockNum := int64(0); ; blockNum++ {
-        blockSize, err := file.Read(buffer)
-        if err == io.EOF {
-            break
-        }
-        if err != nil {
-            dumpError(err)
-            return
-        }
-        block := bytes.NewReader(buffer[:blockSize])
-        blockID := strconv.FormatInt(blockNum, 36)
-        log.Printf("Uploading block %d (size=%d, id=%s).\n",
-            blockNum, blockSize, blockID)
-        err = storage.PutBlock(container, filename, blockID, block)
-        if err != nil {
-            dumpError(err)
-            return
-        }
-        blockList.Add(gwacl.BlockListLatest, blockID)
+    err = storage.UploadBlockBlob(container, filename, file)
+    if err != nil {
+        dumpError(err)
+        return
     }
-
-    // Commit those blocks by writing the block list.
-    log.Printf("Committing %d blocks.\n", len(blockList.Items))
-    err = storage.PutBlockList(container, filename, blockList)
-    dumpError(err)
 }
 
 func deleteblob(storage gwacl.StorageContext) {

=== added directory 'logging'
=== added file 'logging/logging.go'
--- logging/logging.go	1970-01-01 00:00:00 +0000
+++ logging/logging.go	2013-04-22 16:24:27 +0000
@@ -0,0 +1,83 @@
+// Copyright 2013 Canonical Ltd.  This software is licensed under the
+// GNU Lesser General Public License version 3 (see the file COPYING).
+
+package logging
+
+import (
+    "log"
+    "os"
+)
+
+const (
+    DEBUG = 10 * (iota + 1)
+    INFO
+    WARN
+    ERROR
+)
+
+var level = WARN
+
+func init() {
+    setLevel(os.Getenv("LOGLEVEL"))
+}
+
+func setLevel(levelName string) {
+    switch levelName {
+    case "DEBUG":
+        level = DEBUG
+    case "INFO":
+        level = INFO
+    case "WARN":
+        level = WARN
+    case "ERROR":
+        level = ERROR
+    }
+}
+
+func Debug(args ...interface{}) {
+    if level <= DEBUG {
+        log.Println(args...)
+    }
+}
+
+func Debugf(format string, args ...interface{}) {
+    if level <= DEBUG {
+        log.Printf(format, args...)
+    }
+}
+
+func Info(args ...interface{}) {
+    if level <= INFO {
+        log.Println(args...)
+    }
+}
+
+func Infof(format string, args ...interface{}) {
+    if level <= INFO {
+        log.Printf(format, args...)
+    }
+}
+
+func Warn(args ...interface{}) {
+    if level <= WARN {
+        log.Println(args...)
+    }
+}
+
+func Warnf(format string, args ...interface{}) {
+    if level <= WARN {
+        log.Printf(format, args...)
+    }
+}
+
+func Error(args ...interface{}) {
+    if level <= ERROR {
+        log.Println(args...)
+    }
+}
+
+func Errorf(format string, args ...interface{}) {
+    if level <= ERROR {
+        log.Printf(format, args...)
+    }
+}

=== added file 'logging/logging_test.go'
--- logging/logging_test.go	1970-01-01 00:00:00 +0000
+++ logging/logging_test.go	2013-04-22 16:24:27 +0000
@@ -0,0 +1,39 @@
+// Copyright 2013 Canonical Ltd.  This software is licensed under the
+// GNU Lesser General Public License version 3 (see the file COPYING).
+
+package logging
+
+import (
+    . "launchpad.net/gocheck"
+)
+
+var originalLevel = level
+
+func restoreLevel() {
+    level = originalLevel
+}
+
+type testLogging struct{}
+
+var _ = Suite(&testLogging{})
+
+func (suite *testLogging) TestSetLevel(c *C) {
+    defer restoreLevel()
+    // The names of the logging constants are recognised arguments to
+    // setLevel().
+    level = -1
+    setLevel("DEBUG")
+    c.Check(level, Equals, DEBUG)
+    setLevel("INFO")
+    c.Check(level, Equals, INFO)
+    setLevel("WARN")
+    c.Check(level, Equals, WARN)
+    setLevel("ERROR")
+    c.Check(level, Equals, ERROR)
+    // Unrecognised arguments are ignored.
+    level = -1
+    setLevel("FOOBAR")
+    c.Check(level, Equals, -1)
+    setLevel("123")
+    c.Check(level, Equals, -1)
+}

=== added file 'storage.go'
--- storage.go	1970-01-01 00:00:00 +0000
+++ storage.go	2013-04-22 16:24:27 +0000
@@ -0,0 +1,47 @@
+// Copyright 2013 Canonical Ltd.  This software is licensed under the
+// GNU Lesser General Public License version 3 (see the file COPYING).
+
+package gwacl
+
+// This file contains higher level operations necessary to work with the Azure
+// file storage API.
+
+import (
+    "bytes"
+    "io"
+    . "launchpad.net/gwacl/logging"
+    "strconv"
+)
+
+// UploadBlockBlob uses PutBlock and PutBlockList API operations to upload
+// arbitrarily large files, 1MB at a time.
+func (context *StorageContext) UploadBlockBlob(
+    container, filename string, data io.Reader) error {
+
+    buffer := make([]byte, 1024*1024) // 1MB buffer
+    blockList := &BlockList{}
+
+    // Upload the file in chunks.
+    for blockNum := int64(0); ; blockNum++ {
+        blockSize, err := data.Read(buffer)
+        if err == io.EOF {
+            break
+        }
+        if err != nil {
+            return err
+        }
+        block := bytes.NewReader(buffer[:blockSize])
+        blockID := strconv.FormatInt(blockNum, 36)
+        Debugf("Uploading block %d (size=%d, id=%s).\n",
+            blockNum, blockSize, blockID)
+        err = context.PutBlock(container, filename, blockID, block)
+        if err != nil {
+            return err
+        }
+        blockList.Add(BlockListLatest, blockID)
+    }
+
+    // Commit those blocks by writing the block list.
+    Debugf("Committing %d blocks.\n", len(blockList.Items))
+    return context.PutBlockList(container, filename, blockList)
+}

=== renamed file 'storage.go' => 'storage_base.go'
=== renamed file 'storage_test.go' => 'storage_base_test.go'
=== added file 'storage_test.go'
--- storage_test.go	1970-01-01 00:00:00 +0000
+++ storage_test.go	2013-04-22 16:24:27 +0000
@@ -0,0 +1,130 @@
+// Copyright 2013 Canonical Ltd.  This software is licensed under the
+// GNU Lesser General Public License version 3 (see the file COPYING).
+
+package gwacl
+
+import (
+    "bytes"
+    "encoding/base64"
+    "fmt"
+    "io/ioutil"
+    . "launchpad.net/gocheck"
+    "net/http"
+    "net/url"
+)
+
+func b64(s string) string {
+    return base64.StdEncoding.EncodeToString([]byte(s))
+}
+
+type TestTransport2Exchange struct {
+    Request  *http.Request
+    Response *http.Response
+    Error    error
+}
+
+// TestTransport2 is used as an http.Client.Transport for testing. The only
+// requirement is that it adhere to the http.RoundTripper interface.
+type TestTransport2 struct {
+    Exchanges     []*TestTransport2Exchange
+    ExchangeCount int
+}
+
+func (t *TestTransport2) AddExchange(response *http.Response, error error) {
+    exchange := TestTransport2Exchange{Response: response, Error: error}
+    t.Exchanges = append(t.Exchanges, &exchange)
+}
+
+func (t *TestTransport2) RoundTrip(req *http.Request) (resp *http.Response, err error) {
+    exchange := t.Exchanges[t.ExchangeCount]
+    t.ExchangeCount += 1
+    exchange.Request = req
+    return exchange.Response, exchange.Error
+}
+
+type testUploadBlockBlob struct{}
+
+var _ = Suite(&testUploadBlockBlob{})
+
+func (suite *testUploadBlockBlob) TestSmallFile(c *C) {
+    context := makeStorageContext()
+    transport := &TestTransport2{}
+    // UploadBlockBlob uses PutBlock to upload the data.
+    transport.AddExchange(makeFakeCreatedResponse(), nil)
+    // UploadBlockBlob then sends the list of blocks with PutBlockList.
+    transport.AddExchange(makeFakeCreatedResponse(), nil)
+    // Plug in the test transport.
+    context.client = &http.Client{Transport: transport}
+    // Upload a random blob of data.
+    data := uploadRandomBlob(c, context, 10)
+    // There were two exchanges.
+    c.Assert(transport.ExchangeCount, Equals, 2)
+    // The first request is a Put Block with the block data.
+    assertBlockSent(c, context, data, b64("0"), transport.Exchanges[0])
+    // The second request is Put Block List to commit the block above.
+    assertBlockListSent(c, context, []string{b64("0")}, transport.Exchanges[1])
+}
+
+func (suite *testUploadBlockBlob) TestLargeFile(c *C) {
+    context := makeStorageContext()
+    transport := &TestTransport2{}
+    // UploadBlockBlob uses PutBlock twice to upload the data.
+    transport.AddExchange(makeFakeCreatedResponse(), nil)
+    transport.AddExchange(makeFakeCreatedResponse(), nil)
+    // UploadBlockBlob then sends the list of blocks with PutBlockList.
+    transport.AddExchange(makeFakeCreatedResponse(), nil)
+    // Plug in the test transport.
+    context.client = &http.Client{Transport: transport}
+    // Upload a large random blob of data.
+    data := uploadRandomBlob(c, context, 1348*1024)
+    // There were three exchanges.
+    c.Assert(transport.ExchangeCount, Equals, 3)
+    // The first two requests are Put Block with chunks of the block data. The
+    // weird looking block IDs are base64 encodings of the strings "0" and "1".
+    assertBlockSent(c, context, data[:1024*1024], b64("0"), transport.Exchanges[0])
+    assertBlockSent(c, context, data[1024*1024:], b64("1"), transport.Exchanges[1])
+    // The second request is Put Block List to commit the block above.
+    assertBlockListSent(c, context, []string{b64("0"), b64("1")}, transport.Exchanges[2])
+}
+
+func makeFakeCreatedResponse() *http.Response {
+    return &http.Response{
+        Status:     fmt.Sprintf("%d", http.StatusCreated),
+        StatusCode: http.StatusCreated,
+        Body:       Empty,
+    }
+}
+
+func uploadRandomBlob(c *C, context *StorageContext, size int) []byte {
+    data := MakeRandomByteSlice(size)
+    err := context.UploadBlockBlob(
+        "MyContainer", "MyFile", bytes.NewReader(data))
+    c.Assert(err, IsNil)
+    return data
+}
+
+func assertBlockSent(
+    c *C, context *StorageContext, data []byte, blockID string, exchange *TestTransport2Exchange) {
+    c.Check(exchange.Request.URL.String(), Equals, fmt.Sprintf(
+        "http://%s.blob.core.windows.net/MyContainer/MyFile"+
+            "?comp=block&blockid=%s",
+        context.Account, url.QueryEscape(blockID)))
+    body, err := ioutil.ReadAll(exchange.Request.Body)
+    c.Check(err, IsNil)
+    c.Check(body, DeepEquals, data)
+}
+
+func assertBlockListSent(
+    c *C, context *StorageContext, blockIDs []string, exchange *TestTransport2Exchange) {
+    c.Check(exchange.Request.URL.String(), Equals, fmt.Sprintf(
+        "http://%s.blob.core.windows.net/MyContainer/MyFile"+
+            "?comp=blocklist", context.Account))
+    body, err := ioutil.ReadAll(exchange.Request.Body)
+    c.Check(err, IsNil)
+    expected := "\n<BlockList>\n"
+    for _, blockID := range blockIDs {
+        expected += "  <Latest>" + blockID + "</Latest>\n"
+    }
+    expected += "</BlockList>"
+    c.Check(string(body), Equals, expected)
+}

=== modified file 'test_helpers.go'
--- test_helpers.go	2013-04-02 08:51:18 +0000
+++ test_helpers.go	2013-04-22 16:24:27 +0000
@@ -4,7 +4,10 @@
 package gwacl
 
 import (
+    "bytes"
     "fmt"
+    "io"
+    "io/ioutil"
     "math/rand"
     "net/http"
     "strings"
@@ -15,6 +18,9 @@
 // Perhaps we can add it to gocheck, or start a testtools-like project.
 const chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890abcdefghijklmnopqrstuvwxyz"
 
+// A Reader and ReadCloser that EOFs immediately.
+var Empty io.ReadCloser = ioutil.NopCloser(bytes.NewReader(nil))
+
 func MakeRandomString(length int) string {
     return string(MakeRandomByteSlice(length))
 }

=== modified file 'x509dispatcher.go'
--- x509dispatcher.go	2013-03-26 06:50:50 +0000
+++ x509dispatcher.go	2013-04-22 16:24:27 +0000
@@ -5,7 +5,7 @@
     "bytes"
     "fmt"
     curl "github.com/andelf/go-curl"
-    "log"
+    . "launchpad.net/gwacl/logging"
     "net/http"
     "net/textproto"
 )
@@ -143,10 +143,10 @@
 
 func performX509CurlRequest(session *x509Session, request *x509Request) (*x509Response, error) {
     if verbose {
-        log.Println("Performing request")
-        log.Println("Request url: " + request.URL)
-        log.Println("Request method: " + request.Method)
-        log.Printf("Request body: %s\n", request.Payload)
+        Debug("Performing request")
+        Debug("Request url: " + request.URL)
+        Debug("Request method: " + request.Method)
+        Debugf("Request body: %s\n", request.Payload)
     }
     response := newX509Response()
     ch := request.makeCurlRequest(session, response)
@@ -170,10 +170,10 @@
     }
     response.StatusCode = status.(int)
     if verbose {
-        log.Println("Got response")
-        log.Printf("Response status: %d\n", response.StatusCode)
-        log.Printf("Response headers: %s\n", response.RawHeader)
-        log.Printf("Response body: %s\n", response.Body)
+        Debug("Got response")
+        Debugf("Response status: %d\n", response.StatusCode)
+        Debugf("Response headers: %s\n", response.RawHeader)
+        Debugf("Response body: %s\n", response.Body)
     }
     return response, nil
 }