added SDB Item and Attribute-level operations

This is a re-submit of the following patch: 
https://codereview.appspot.com/5451067/

Made the following changes as per the code review
1) improved struct Item documentation
2) changed Attribute to Attr, and only has Name / Value fields
3) dropped ListDomainsOpt struct since it is unused
4) split ListDomains into ListDomains and ListDomainsN
5) improved ListDomains and ListDomainsN comments per suggestions
6) changed ListDomainsN input maxNumberOfDomains to maxDomains
7) changes struct SelectResponse to SelectResp
8) changed SelectResp.Item to SelectResp.Items and Attributes to Attrs
9) changed Select func input param consistentRead to consistent
10) dropped DeleteDomainOpt struct since it is unused
11) implemented PutAttrs per recommendations
12) updated unit tests to reflect all changes, and tested against live AWS to ensure everything works as expected
13) ran gofmt

https://code.launchpad.net/~brad-rydzewski/goamz/sdb-item-operations/+merge/92570

(do not edit description out of merge proposal)

[Brad.Rydzewski, 2012-02-10 20:33:07]

Please take a look.

[Brad.Rydzewski, 2012-02-10 20:44:38]

On 2012/02/10 20:33:07, Brad.Rydzewski wrote:
> Please take a look.

Please note that I did not include the DomainMetadata functions

So once this patch is approved, the SDB library will enapsulate all but the
following operations:
DomainMetadata
BatchPutAttributes
DeleteAttributes
BatchDeleteAttributes

I will add DeleteAttributes next, which is simliar to PutAttributes. So i'm
wondering if you think we should rename the PutAttrs struct to something more
generic?

[niemeyer, 2012-03-05 01:46:11]

This is looking very good. There's only one minor detail in the model that I'm
not quite sure about the best way to solve yet, but I'll figure something out.
If you sort out these issues, we can either integrate, or I'll provide some
further feedback on that one point.

I really apologize for the delay on this one.

https://codereview.appspot.com/5649056/diff/1/sdb.go
File sdb.go (right):

https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode39
sdb.go:39: // by attribute-value pairs.
s/attribute-value pairs/name-value attributes/

https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode50
sdb.go:50: // The Item type represent individual objects that contain one or
more attribute
s/represent/represents/
s/attribute name-value pairs/name-value attributes/

https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode66
sdb.go:66: Value string // The value of this attribute
Please drop line comments for Name and Value. They are implicit from the field
names.

https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode116
sdb.go:116: }                `xml:"SelectResult>Item"`
This doesn't smell right. There's something not quite ideal in the model. We
have an Item, we have Attr, and as a response we're getting Attrs in something
that is not an Item, and a loose Name.

Please let me think about that one while you sort out the rest.

https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode120
sdb.go:120: // Select returns a set of attributes for item names that match the
select expression.
s/the select expression/expr/

https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode164
sdb.go:164: Attrs    []Attr // The attributes to put
Why is this public while everything else is not?

Also, would you mind to drop line comment (Attrs in PutAttrs can only be attrs
to put).

https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode182
sdb.go:182: // The PutAttrs request will only success if the existing
s/success/succeed/

https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode183
sdb.go:183: // Item in SimpleDB contains a matching name / value pair.
s/Item/item/.. this isn't talking about the type.

https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode185
sdb.go:185: // This should set Expected.N.Name=name and Expected.N.Value=value
Please drop this comment, as we don't have that kind of documentation in other
places.

https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode201
sdb.go:201: // PutAttrs adds attrs to an item.
s/an item/item/ (referring to the receiver)

https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode244
sdb.go:244: // Response to a GetAttrs request.
s/GetAttrs/Attrs/, here and below please. Accessors in Go generally don't have
the Get.

https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode252
sdb.go:252: // GetAttrs returns all of the attrs associated with the item.
s/the item/item/

https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode260
sdb.go:260: // GetAttrsSelect returns one or more specified attr name
parameters.
Ditto.

https://codereview.appspot.com/5649056/diff/1/sdb_test.go
File sdb_test.go (right):

https://codereview.appspot.com/5649056/diff/1/sdb_test.go#newcode6
sdb_test.go:6: "launchpad.net/gocheck"
Please import this as:

    . "launchpad.net/gocheck"

So that you can avoid the excessive use of the "gocheck." prefix.

https://codereview.appspot.com/5649056/diff/1/sdb_test.go#newcode68
sdb_test.go:68: c.Assert(resp.Domains, gocheck.Equals,
[]string{"Domain1-200706011651", "Domain2-200706011652"})
You'll need to port to the current gocheck, using DeepEquals here.

https://codereview.appspot.com/5649056/diff/1/sdb_test.go#newcode109
sdb_test.go:109: c.Assert(req.Form["Action"], gocheck.Equals,
[]string{"PutAttributes"})
All of these have to be DeepEquals.

https://codereview.appspot.com/5649056/diff/1/sdb_test.go#newcode142
sdb_test.go:142: c.Assert(req.Form["Action"], gocheck.Equals,
[]string{"GetAttributes"})
DeepEquals here and below too.

https://codereview.appspot.com/5649056/diff/1/sdb_test.go#newcode169
sdb_test.go:169: c.Assert(req.Form["Action"], gocheck.Equals,
[]string{"GetAttributes"})
Ditto.

[Brad.Rydzewski, 2012-03-05 17:42:45]

Please take a look.

[Brad.Rydzewski, 2012-03-05 17:47:01]

On 2012/03/05 01:46:11, niemeyer wrote:
> This is looking very good. There's only one minor detail in the model that I'm
> not quite sure about the best way to solve yet, but I'll figure something out.
> If you sort out these issues, we can either integrate, or I'll provide some
> further feedback on that one point.
> 
> I really apologize for the delay on this one.
> 
> https://codereview.appspot.com/5649056/diff/1/sdb.go
> File sdb.go (right):
> 
> https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode39
> sdb.go:39: // by attribute-value pairs.
> s/attribute-value pairs/name-value attributes/
> 
> https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode50
> sdb.go:50: // The Item type represent individual objects that contain one or
> more attribute
> s/represent/represents/
> s/attribute name-value pairs/name-value attributes/
> 
> https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode66
> sdb.go:66: Value string // The value of this attribute
> Please drop line comments for Name and Value. They are implicit from the field
> names.
> 
> https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode116
> sdb.go:116: }                `xml:"SelectResult>Item"`
> This doesn't smell right. There's something not quite ideal in the model. We
> have an Item, we have Attr, and as a response we're getting Attrs in something
> that is not an Item, and a loose Name.
> 
> Please let me think about that one while you sort out the rest.
> 
> https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode120
> sdb.go:120: // Select returns a set of attributes for item names that match
the
> select expression.
> s/the select expression/expr/
> 
> https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode164
> sdb.go:164: Attrs    []Attr // The attributes to put
> Why is this public while everything else is not?
> 
> Also, would you mind to drop line comment (Attrs in PutAttrs can only be attrs
> to put).
> 
> https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode182
> sdb.go:182: // The PutAttrs request will only success if the existing
> s/success/succeed/
> 
> https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode183
> sdb.go:183: // Item in SimpleDB contains a matching name / value pair.
> s/Item/item/.. this isn't talking about the type.
> 
> https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode185
> sdb.go:185: // This should set Expected.N.Name=name and Expected.N.Value=value
> Please drop this comment, as we don't have that kind of documentation in other
> places.
> 
> https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode201
> sdb.go:201: // PutAttrs adds attrs to an item.
> s/an item/item/ (referring to the receiver)
> 
> https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode244
> sdb.go:244: // Response to a GetAttrs request.
> s/GetAttrs/Attrs/, here and below please. Accessors in Go generally don't have
> the Get.
> 
> https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode252
> sdb.go:252: // GetAttrs returns all of the attrs associated with the item.
> s/the item/item/
> 
> https://codereview.appspot.com/5649056/diff/1/sdb.go#newcode260
> sdb.go:260: // GetAttrsSelect returns one or more specified attr name
> parameters.
> Ditto.
> 
> https://codereview.appspot.com/5649056/diff/1/sdb_test.go
> File sdb_test.go (right):
> 
> https://codereview.appspot.com/5649056/diff/1/sdb_test.go#newcode6
> sdb_test.go:6: "launchpad.net/gocheck"
> Please import this as:
> 
>     . "launchpad.net/gocheck"
> 
> So that you can avoid the excessive use of the "gocheck." prefix.
> 
> https://codereview.appspot.com/5649056/diff/1/sdb_test.go#newcode68
> sdb_test.go:68: c.Assert(resp.Domains, gocheck.Equals,
> []string{"Domain1-200706011651", "Domain2-200706011652"})
> You'll need to port to the current gocheck, using DeepEquals here.
> 
> https://codereview.appspot.com/5649056/diff/1/sdb_test.go#newcode109
> sdb_test.go:109: c.Assert(req.Form["Action"], gocheck.Equals,
> []string{"PutAttributes"})
> All of these have to be DeepEquals.
> 
> https://codereview.appspot.com/5649056/diff/1/sdb_test.go#newcode142
> sdb_test.go:142: c.Assert(req.Form["Action"], gocheck.Equals,
> []string{"GetAttributes"})
> DeepEquals here and below too.
> 
> https://codereview.appspot.com/5649056/diff/1/sdb_test.go#newcode169
> sdb_test.go:169: c.Assert(req.Form["Action"], gocheck.Equals,
> []string{"GetAttributes"})
> Ditto.

I think this includes all suggested changes. Regarding SelectResp ... i'm sure
there is a better way to represent this. It is my recommendation, however, that
we fix this as a separate patch, since trunk is not in a working state and
people are working off of this branch. I'd be happy to patch once you have some
ideas about how to clean up

[niemeyer, 2012-03-05 18:22:29]

On 2012/03/05 17:47:01, Brad.Rydzewski wrote:
> I think this includes all suggested changes. Regarding SelectResp ... i'm sure
> there is a better way to represent this. It is my recommendation, however,
that
> we fix this as a separate patch, since trunk is not in a working state and
> people are working off of this branch. I'd be happy to patch once you have
some
> ideas about how to clean up

Sounds good. We can put this in exp/sdb for the moment, to make sure people
realize it's in experimental state and will have a few fast-paced changed in the
coming weeks.

I also want to get everything in as we'll move onto a one-branch-for-all model
for the goamz packages. It'll be way easier than using all the separate series
as we're doing now.

I'll have a look at this and integrate.

[niemeyer, 2012-03-09 10:29:12]

*** Submitted:

sdb: added item and attribute-level operations

This is a re-submit of the following patch: 
https://codereview.appspot.com/5451067/

Made the following changes as per the code review

1) improved struct Item documentation
2) changed Attribute to Attr, and only has Name / Value fields
3) dropped ListDomainsOpt struct since it is unused
4) split ListDomains into ListDomains and ListDomainsN
5) improved ListDomains and ListDomainsN comments per suggestions
6) changed ListDomainsN input maxNumberOfDomains to maxDomains
7) changes struct SelectResponse to SelectResp
8) changed SelectResp.Item to SelectResp.Items and Attributes to Attrs
9) changed Select func input param consistentRead to consistent
10) dropped DeleteDomainOpt struct since it is unused
11) implemented PutAttrs per recommendations
12) updated unit tests to reflect all changes, and tested against
    live AWS to ensure everything works as expected
13) ran gofmt

R=niemeyer
CC=
https://codereview.appspot.com/5649056