New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comparing S3 packages #11

Open
nathany opened this Issue Jan 4, 2015 · 8 comments

Comments

Projects
None yet
3 participants
@nathany
Contributor

nathany commented Jan 4, 2015

A surface level comparison of:

mitchellh/goamz/s3

mitchellh/goamz/s3 doesn't have func RetryAttempts(retry bool) but there are several added methods:

func (b *Bucket) Copy(oldPath, newPath string, perm ACL) error
func (b *Bucket) GetBucketContents() (*map[string]Key, error)
func (b *Bucket) GetKey(path string) (*Key, error)
func (b *Bucket) GetResponse(path string) (*http.Response, error)
func (b *Bucket) GetTorrent(path string) ([]byte, error)
func (b *Bucket) GetTorrentReader(path string) (io.ReadCloser, error)
func (b *Bucket) Head(path string) (*http.Response, error)
func (b *Bucket) MultiDel(paths []string) error
func (b *Bucket) PutHeader(path string, data []byte, customHeaders map[string][]string, perm ACL) error
func (b *Bucket) PutReaderHeader(path string, r io.Reader, length int64, customHeaders map[string][]string, perm ACL) error
func (s3 *S3) ListBuckets() (result *ListBucketsResp, err error)

crowdmob/goamz/s3

crowdmob/goamz/s3 has added some of the same methods as mitchellh/goamz/s3

func (b *Bucket) GetResponse(path string) (resp *http.Response, err error)

and others, but with a different method signature:

func (b *Bucket) Head(path string, headers map[string][]string) (*http.Response, error)

it has also changed some of the amz.v1 methods to take an Options struct:

func (b *Bucket) InitMulti(key string, contType string, perm ACL, options Options) (*Multi, error)
func (b *Bucket) Put(path string, data []byte, contType string, perm ACL, options Options) error
func (b *Bucket) PutReader(path string, r io.Reader, length int64, contType string, perm ACL, options Options) error

and then added a whole slough of new methods, some that sound like they accomplish the same things as mitchellh/goamz/s3, but in a different way:

func (b *Bucket) DelMulti(objects Delete) error
func (b *Bucket) DeleteLifecycleConfiguration() error
func (b *Bucket) Exists(path string) (exists bool, err error)
func (b *Bucket) GetLifecycleConfiguration() (*LifecycleConfiguration, error)
func (b *Bucket) GetResponseWithHeaders(path string, headers map[string][]string) (resp *http.Response, err error)
func (b *Bucket) Location() (string, error)
func (b *Bucket) PostFormArgs(path string, expires time.Time, redirect string) (action string, fields map[string]string)
func (b *Bucket) PostFormArgsEx(path string, expires time.Time, redirect string, conds []string) (action string, fields map[string]string)
func (b *Bucket) PutBucketSubresource(subresource string, r io.Reader, length int64) error
func (b *Bucket) PutBucketWebsite(configuration WebsiteConfiguration) error
func (b *Bucket) PutCopy(path string, perm ACL, options CopyOptions, source string) (*CopyObjectResult, error)
func (b *Bucket) PutLifecycleConfiguration(c *LifecycleConfiguration) error
func (b *Bucket) SignedURLWithArgs(path string, expires time.Time, params url.Values, headers http.Header) string
func (b *Bucket) UploadSignedURL(path, method, content_type string, expires time.Time) string
func (b *Bucket) Versions(prefix, delim, keyMarker string, versionIdMarker string, max int) (result *VersionsResp, err error)
# ... and a whole lot more (those are just under bucket)

goamz/goamz/s3 looks pretty similar to crowdmob/goamz/s3.

This isn't to say all these methods should be added. But it does mean I can't take even a trivial project that was using mitchellh/goamz/s3 and recompile it with amz.v1/s3. Handy methods like GetBucketContents and MultiDel don't exist.

As another point of comparison, there is Stripe's generated API:
http://godoc.org/github.com/stripe/aws-go/gen/s3

Note: I can't currently refer to http://godoc.org/gopkg.in/amz.v2-dev/s3 for comparison.

@dimitern

This comment has been minimized.

Show comment
Hide comment
@dimitern

dimitern Jan 5, 2015

Member

Thanks @nathany for the summary! I'll have a deeper look, but I can say there are no changes to the s3 package between v1 and v2-dev.

Member

dimitern commented Jan 5, 2015

Thanks @nathany for the summary! I'll have a deeper look, but I can say there are no changes to the s3 package between v1 and v2-dev.

@nathany

This comment has been minimized.

Show comment
Hide comment
@nathany

nathany Jan 8, 2015

Contributor

How to proceed?

Contributor

nathany commented Jan 8, 2015

How to proceed?

@dimitern

This comment has been minimized.

Show comment
Hide comment
@dimitern

dimitern Jan 8, 2015

Member

@matrixik, @nathany Hey guys, I had a chat with @niemeyer about those and other related issues. I'll send more details later today. There will be a CONTRIBUTING.md file with guidelines, and basically everyone who's familiar with both EC2/AWS and goamz/Go and wants to help and cares about the project can (and should) be a maintainer. Initially, the maintainers will be myself, @axw, @rogpeppe, @bz2, @katco-, and @wallyworld. More later, as I have to do a write-up.

Member

dimitern commented Jan 8, 2015

@matrixik, @nathany Hey guys, I had a chat with @niemeyer about those and other related issues. I'll send more details later today. There will be a CONTRIBUTING.md file with guidelines, and basically everyone who's familiar with both EC2/AWS and goamz/Go and wants to help and cares about the project can (and should) be a maintainer. Initially, the maintainers will be myself, @axw, @rogpeppe, @bz2, @katco-, and @wallyworld. More later, as I have to do a write-up.

@dimitern

This comment has been minimized.

Show comment
Hide comment
@dimitern

dimitern Jan 12, 2015

Member

I think we should try to integrate those by carefully pulling in the common super set of methods - branch v3-unstable can be used as a development focus.

Member

dimitern commented Jan 12, 2015

I think we should try to integrate those by carefully pulling in the common super set of methods - branch v3-unstable can be used as a development focus.

@nathany

This comment has been minimized.

Show comment
Hide comment
@nathany

nathany Jan 12, 2015

Contributor

Thanks for adding the CONTRIBUTING and getting the unstable branch setup.

For existing packages like this, I'm not sure if it makes sense to try to maintain the commits and who did them when bringing over methods. It would be nice, but seems like a lot of effort.

Instead it may make sense just to bring over missing functionality with the same method signatures -- or different if the API can be improved upon.

What are your thoughts?

Contributor

nathany commented Jan 12, 2015

Thanks for adding the CONTRIBUTING and getting the unstable branch setup.

For existing packages like this, I'm not sure if it makes sense to try to maintain the commits and who did them when bringing over methods. It would be nice, but seems like a lot of effort.

Instead it may make sense just to bring over missing functionality with the same method signatures -- or different if the API can be improved upon.

What are your thoughts?

@dimitern

This comment has been minimized.

Show comment
Hide comment
@dimitern

dimitern Jan 12, 2015

Member

@nathany All that sounds good to me. We should try to keep the code consistent though, in naming, doc comments on exported items, etc. Also a potential thing that might slow down some PRs will be to converts tests not using gocheck.

Member

dimitern commented Jan 12, 2015

@nathany All that sounds good to me. We should try to keep the code consistent though, in naming, doc comments on exported items, etc. Also a potential thing that might slow down some PRs will be to converts tests not using gocheck.

@nathany

This comment has been minimized.

Show comment
Hide comment
@nathany

nathany Jan 13, 2015

Contributor

okay. that should be no problem, golint lights up in my editor when I fail to comply.

Contributor

nathany commented Jan 13, 2015

okay. that should be no problem, golint lights up in my editor when I fail to comply.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment