cloud/storage: implementing object listing

cloud/storage: implementing object listing

[adg, 2014-09-22 05:07:02]

https://codereview.appspot.com/144290043/diff/40001/storage/object.go File storage/object.go (right): https://codereview.appspot.com/144290043/diff/40001/storage/object.go#newcode150 storage/object.go:150: // Delimeter returns results in a directory-like fashion. This doc comment is not clear. Are each of these fields optional? Delimiter is a very weird name. I don't even know how I might use the Delimiter and Prefix fields here. Perhaps we're lacking an example in the Query doc comment https://codereview.appspot.com/144290043/diff/40001/storage/object.go#newcode173 storage/object.go:173: MaxResults uint64 This uint64 seems... excessive. Is it part of the underlying API? https://codereview.appspot.com/144290043/diff/40001/storage/object.go#newcode176 storage/object.go:176: // Objects represent a list of objects returned from represents

[jbd, 2014-09-22 18:50:05]

https://codereview.appspot.com/144290043/diff/40001/storage/object.go File storage/object.go (right): https://codereview.appspot.com/144290043/diff/40001/storage/object.go#newcode150 storage/object.go:150: // Delimeter returns results in a directory-like fashion. On 2014/09/22 05:07:02, adg wrote: > This doc comment is not clear. > Are each of these fields optional? > Delimiter is a very weird name. > I don't even know how I might use the Delimiter and Prefix fields here. > Perhaps we're lacking an example in the Query doc comment Each of these fields is optional. Delimiter and prefix are both industry standard terms. http://docs.openstack.org/api/openstack-object-storage/1.0/content/pseudo-hie... I think user will look for API docs if they don't understand what they actually stand for. I don't want to document them, because it requires extensive documentation with examples, etc. It's a shame that GCS docs don't have a page for further explanation we can link to :( I will address the other issues.

[jbd, 2014-09-22 22:36:12]

https://codereview.appspot.com/144290043/diff/40001/storage/object.go File storage/object.go (right): https://codereview.appspot.com/144290043/diff/40001/storage/object.go#newcode150 storage/object.go:150: // Delimeter returns results in a directory-like fashion. On 2014/09/22 05:07:02, adg wrote: > This doc comment is not clear. > Are each of these fields optional? > Delimiter is a very weird name. > I don't even know how I might use the Delimiter and Prefix fields here. > Perhaps we're lacking an example in the Query doc comment Done. https://codereview.appspot.com/144290043/diff/40001/storage/object.go#newcode173 storage/object.go:173: MaxResults uint64 On 2014/09/22 05:07:02, adg wrote: > This uint64 seems... excessive. A bit, fixing. Done. https://codereview.appspot.com/144290043/diff/40001/storage/object.go#newcode176 storage/object.go:176: // Objects represent a list of objects returned from On 2014/09/22 05:07:02, adg wrote: > represents Done.

[adg, 2014-09-23 01:37:31]

LGTM https://codereview.appspot.com/144290043/diff/60001/storage/object.go File storage/object.go (right): https://codereview.appspot.com/144290043/diff/60001/storage/object.go#newcode166 storage/object.go:166: // Optional. makes no sense on a bool https://codereview.appspot.com/144290043/diff/60001/storage/object.go#newcode177 storage/object.go:177: // Optional. instead of saying optional, indicate that zero means no limit https://codereview.appspot.com/144290043/diff/60001/storage/object.go#newcode178 storage/object.go:178: MaxResults uint i'd probably make this just an int; negative values could just mean the same as zero. this makes it less likely to need an awkward type conversion where it is used https://codereview.appspot.com/144290043/diff/60001/storage/object.go#newcode182 storage/object.go:182: // a bucket look-up request and a query to retrieve more lookup https://codereview.appspot.com/144290043/diff/60001/storage/storage.go File storage/storage.go (right): https://codereview.appspot.com/144290043/diff/60001/storage/storage.go#newcod... storage/storage.go:119: c.Delimiter(q.Delimeter) should these be testing for empty string etc?

[jbd, 2014-09-23 04:36:58]

https://codereview.appspot.com/144290043/diff/60001/storage/object.go File storage/object.go (right): https://codereview.appspot.com/144290043/diff/60001/storage/object.go#newcode166 storage/object.go:166: // Optional. On 2014/09/23 01:37:30, adg wrote: > makes no sense on a bool Done. https://codereview.appspot.com/144290043/diff/60001/storage/object.go#newcode177 storage/object.go:177: // Optional. On 2014/09/23 01:37:31, adg wrote: > instead of saying optional, indicate that zero means no limit Done. zero or less = API default https://codereview.appspot.com/144290043/diff/60001/storage/object.go#newcode178 storage/object.go:178: MaxResults uint On 2014/09/23 01:37:30, adg wrote: > i'd probably make this just an int; negative values could just mean the same as > zero. this makes it less likely to need an awkward type conversion where it is > used Done. https://codereview.appspot.com/144290043/diff/60001/storage/object.go#newcode182 storage/object.go:182: // a bucket look-up request and a query to retrieve more On 2014/09/23 01:37:31, adg wrote: > lookup Done. https://codereview.appspot.com/144290043/diff/60001/storage/storage.go File storage/storage.go (right): https://codereview.appspot.com/144290043/diff/60001/storage/storage.go#newcod... storage/storage.go:119: c.Delimiter(q.Delimeter) On 2014/09/23 01:37:31, adg wrote: > should these be testing for empty string etc? The API ignores empty strings, even though it uses them for filtering, the end result would not change. But it ignores, so we don't have a perf problem by adding them.

[jbd, 2014-09-23 04:38:38]

Submitted as 69ab593ddd707905b5c35c5fb161803de1b38c22.