Code review comment for lp:~niemeyer/goamz/put-all
Revision history for this message
| Gustavo Niemeyer (niemeyer) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
a60fb26
(Get the code!)
Thanks for the reviews.
https:/
File s3/multi.go (right):
https:/
s3/multi.go:227: var listPartsMax = 1000
On 2013/02/12 08:02:52, dimitern wrote:
> Is this an absolute limit of all parts per client connection?
No, this is a per-request limit, and 1000 is the default limit as well.
It's here as a variable just so we can test it. I'll add a comment.
https:/
s3/multi.go:272: // the new part, or otherwise overwritten with the new
content.
On 2013/02/12 07:32:38, jameinel wrote:
> does partSize need to match exactly the pre-uploaded content? I don't
see places
> where that is checked here. Probably at least a doc comment about it.
It's actually explicitly checked, but that's a bit redundant with the
above statement. If the size changes, the checksum won't match. That
said, given that you felt the need to ask, it won't hurt to mention it
explicitly.
https:/
s3/multi.go:282: if totalSize == 0 || err != nil {
On 2013/02/12 07:32:38, jameinel wrote:
> So I would probably split this:
> if err != nil {
> return nil, err
> } else if totalSize == 0 {
> return nil, nil
> }
Hmm.. that does exactly the same thing that the current version does.
> Though since you don't have a result object, wouldn't you want a
non-nil err?
> Otherwise you'll get a segfault accessing the result later.
That's not the case with slices. The most conventional way to represent
an empty slice is with a nil/zero value. len, append, etc, all work
properly with them, and code taking slices should behave properly as
well, or may otherwise be considered broken.
https:/
s3/multi.go:310: part, err := m.putPart(current, section, partSize,
md5b64)
On 2013/02/12 07:32:38, jameinel wrote:
> As an aside, would there be any reason to do parallel uploads?
That's an interesting question. I guess we could probably get some speed
up by sending parts in parallel. It's a good topic to do some
experimentation on.
https:/