Add Aliyun OSS storage option. #419

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
2 participants
@huacnlee

Aliyun OSS is one of most used storage solutions in China, like S3.

We need store files in local storage host, it will faster that Dropbox, S3...

This pull request add :aliyun option for storage.

Usage:

store_with Aliyun do |aliyun|
  aliyun.access_key_id = 'my_access_id'
  aliyun.access_key_secret = 'my_access_key'
  aliyun.bucket = 'foo'
  aliyun.path = ''
  aliyun.keep = 2
end
@benmccann

This comment has been minimized.

Show comment Hide comment
@benmccann

benmccann Mar 12, 2013

I think it might be better to call the class Aliyun or AliyunOSS instead of OSS.

Also, you will need to add a template under templates/cli/storage

I think it might be better to call the class Aliyun or AliyunOSS instead of OSS.

Also, you will need to add a template under templates/cli/storage

@huacnlee

This comment has been minimized.

Show comment Hide comment
@huacnlee

huacnlee Mar 13, 2013

@benmccann Storage name changed.

@benmccann Storage name changed.

@benmccann

This comment has been minimized.

Show comment Hide comment
@benmccann

benmccann Mar 13, 2013

You probably have to change spec/cli_spec.rb as well

You probably have to change spec/cli_spec.rb as well

@huacnlee

This comment has been minimized.

Show comment Hide comment
@huacnlee

huacnlee Mar 14, 2013

@benmccann cli test has fixed.

@benmccann cli test has fixed.

@ghost

View changes

lib/backup/storage/aliyun.rb
+ class Aliyun < Base
+ attr_accessor :bucket,:access_key_id,:access_key_secret, :path
+
+ attr_deprecate "carrierwave-aliyun", :version => '>= 0.1.3'

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Apr 5, 2013

???

This comment has been minimized.

Show comment Hide comment
@huacnlee

huacnlee Apr 7, 2013

Because it need that to access Aliyun OSS

@huacnlee

huacnlee Apr 7, 2013

Because it need that to access Aliyun OSS

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Apr 7, 2013

Ok. Will look into it as soon as I get a chance.

@ghost

ghost Apr 7, 2013

Ok. Will look into it as soon as I get a chance.

This comment has been minimized.

Show comment Hide comment
@huacnlee

huacnlee Sep 12, 2013

Hi @burns , Why did not merge this pull request.
We want this feature to store backup files in locally Cloud hosting, to instead store my backup in Dropbox, because our Web Host in China connection to Dropbox is so hard, so slow.

And I want to write the another Cloud hosting for Backup store interface.

Would you please tell me the reason of not merge this pull request in you free time? and then I can fix it.

@huacnlee

huacnlee Sep 12, 2013

Hi @burns , Why did not merge this pull request.
We want this feature to store backup files in locally Cloud hosting, to instead store my backup in Dropbox, because our Web Host in China connection to Dropbox is so hard, so slow.

And I want to write the another Cloud hosting for Backup store interface.

Would you please tell me the reason of not merge this pull request in you free time? and then I can fix it.

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Sep 12, 2013

All Storages, including their tests, have been updated since this PR. So it needs to be re-written.
But I would still like for you to explain the purpose of Line#8.

@ghost

ghost Sep 12, 2013

All Storages, including their tests, have been updated since this PR. So it needs to be re-written.
But I would still like for you to explain the purpose of Line#8.

This comment has been minimized.

Show comment Hide comment
@huacnlee

huacnlee Sep 13, 2013

Line#8 is a mistake use, I will fix it.

@huacnlee

huacnlee Sep 13, 2013

Line#8 is a mistake use, I will fix it.

@huacnlee

This comment has been minimized.

Show comment Hide comment
@huacnlee

huacnlee Sep 13, 2013

@burns New fixes has committed, and this has it test in stroage/aliyun_spec.rb

@burns New fixes has committed, and this has it test in stroage/aliyun_spec.rb

@huacnlee

This comment has been minimized.

Show comment Hide comment
@huacnlee

huacnlee Sep 13, 2013

Not need merge this pull_request now, I found a new way to solution this.

I write to plugin for Backup

Just only install them and require in Backup model file.

Not need merge this pull_request now, I found a new way to solution this.

I write to plugin for Backup

Just only install them and require in Backup model file.

@huacnlee huacnlee closed this Sep 13, 2013

@ghost

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Sep 13, 2013

OK. Just so you're aware, calling file.read could consume an enormous amount of memory.
And after reviewing rest-client, even if you were to only pass the file as the "payload",
rest-client will simply do the same (call #read). It's simply not designed for large file uploads.

ghost commented Sep 13, 2013

OK. Just so you're aware, calling file.read could consume an enormous amount of memory.
And after reviewing rest-client, even if you were to only pass the file as the "payload",
rest-client will simply do the same (call #read). It's simply not designed for large file uploads.

@huacnlee

This comment has been minimized.

Show comment Hide comment
@huacnlee

huacnlee Sep 13, 2013

@burns It's have to use file.read , because Aliyun OSS API need send file content MD5 digest and Content-Length, so...

@burns It's have to use file.read , because Aliyun OSS API need send file content MD5 digest and Content-Length, so...

@huacnlee

This comment has been minimized.

Show comment Hide comment
@huacnlee

huacnlee Sep 13, 2013

Digest::MD5.file(File.open("xxx")) can instead file.read to MD5, now large file no problem.

Digest::MD5.file(File.open("xxx")) can instead file.read to MD5, now large file no problem.

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