Skip to content
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

Add Backblaze B2 Cloud Storage #410

Merged
merged 1 commit into from
Dec 14, 2020
Merged

Conversation

duartemvix
Copy link
Contributor

This should add support for Backblaze B2 Cloud Storage to Asset Sync

@duartemvix duartemvix force-pushed the backblazeb2 branch 2 times, most recently from e4909b9 to e971775 Compare December 11, 2020 02:54
@PikachuEXE
Copy link
Member

Have you tested this with actual Backblaze B2?

@duartemvix
Copy link
Contributor Author

Have you tested this with actual Backblaze B2?

Yes, I did. I'm already using it in my other repositories linkng to my own fork. This one on this PR.

@coveralls
Copy link

coveralls commented Dec 11, 2020

Pull Request Test Coverage Report for Build 478

  • 9 of 35 (25.71%) changed or added relevant lines in 5 files are covered.
  • 3 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-2.3%) to 63.509%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/asset_sync/storage.rb 2 3 66.67%
lib/asset_sync/engine.rb 0 3 0.0%
lib/asset_sync/config.rb 7 11 63.64%
lib/generators/asset_sync/templates/asset_sync.rb 0 5 0.0%
lib/generators/asset_sync/install_generator.rb 0 13 0.0%
Files with Coverage Reduction New Missed Lines %
lib/asset_sync/config.rb 1 92.15%
lib/asset_sync/engine.rb 1 0%
lib/asset_sync/storage.rb 1 72.63%
Totals Coverage Status
Change from base Build 470: -2.3%
Covered Lines: 409
Relevant Lines: 644

💛 - Coveralls

@PikachuEXE
Copy link
Member

There are some failed jobs if you look into Travis
Can you fix it?

@duartemvix
Copy link
Contributor Author

There are some failed jobs if you look into Travis
Can you fix it?

At Travis, apparently it's failing on old Ruby versions. And as far as I've seen it I'm not sure what is really wrong. I can't seem to understand Coveralls too.

@PikachuEXE
Copy link
Member

I think it's due to the double created not stubbing all the method calls

it 'deletes files sequentially' do
remote_files = ['public/image.png']
connection = double
config = double
directories = double
directory = double
file = double
storage = AssetSync::Storage.new(@config)
[:local_files, :ignored_files, :always_upload_files].each do |method|
expect(storage).to receive(method).and_return([])
end
allow(storage).to receive(:get_remote_files).and_return(remote_files)
allow(storage).to receive(:connection).and_return(connection).twice
allow(storage).to receive(:config).and_return(config)
allow(config).to receive(:aws?).and_return(false)
allow(config).to receive(:fog_directory).and_return('foo')
allow(config).to receive(:assets_prefix).and_return('foo')
allow(directories).to receive(:get).and_return(directory)
allow(directory).to receive(:files).and_return([file])
allow(file).to receive(:key).and_return('public/image.png')
allow(connection).to receive(:directories).and_return(directories)
expect(connection).not_to receive(:delete_multiple_objects)
expect(file).to receive(:destroy)
storage.delete_extra_remote_files

In this case your updated code calls a new method backblaze? which is not stubbed
Adding a line in that test case like

allow(config).to receive(:backblaze?).and_return(false)

should fix it

@duartemvix
Copy link
Contributor Author

I think it's due to the double created not stubbing all the method calls

it 'deletes files sequentially' do
remote_files = ['public/image.png']
connection = double
config = double
directories = double
directory = double
file = double
storage = AssetSync::Storage.new(@config)
[:local_files, :ignored_files, :always_upload_files].each do |method|
expect(storage).to receive(method).and_return([])
end
allow(storage).to receive(:get_remote_files).and_return(remote_files)
allow(storage).to receive(:connection).and_return(connection).twice
allow(storage).to receive(:config).and_return(config)
allow(config).to receive(:aws?).and_return(false)
allow(config).to receive(:fog_directory).and_return('foo')
allow(config).to receive(:assets_prefix).and_return('foo')
allow(directories).to receive(:get).and_return(directory)
allow(directory).to receive(:files).and_return([file])
allow(file).to receive(:key).and_return('public/image.png')
allow(connection).to receive(:directories).and_return(directories)
expect(connection).not_to receive(:delete_multiple_objects)
expect(file).to receive(:destroy)
storage.delete_extra_remote_files

In this case your updated code calls a new method backblaze? which is not stubbed
Adding a line in that test case like

allow(config).to receive(:backblaze?).and_return(false)

should fix it

I missed that file. You're right, I'll push new changes right about now.

@duartemvix duartemvix force-pushed the backblazeb2 branch 3 times, most recently from 28780a1 to 296615a Compare December 11, 2020 15:47
@duartemvix
Copy link
Contributor Author

duartemvix commented Dec 12, 2020

@PikachuEXE Have any idea on how to fix jobs that failed running on jruby?

@PikachuEXE
Copy link
Member

Don't worry
The "error" is

The job exceeded the maximum time limit for jobs, and has been terminated.

I will just ignore jruby result, probably all passed
And this "error" is probably TravisCI issue

Merging next week otherwise I will forget to release (Too lazy to bring up my Mac on weekend...)

@duartemvix
Copy link
Contributor Author

duartemvix commented Dec 13, 2020

Don't worry
The "error" is

The job exceeded the maximum time limit for jobs, and has been terminated.

I will just ignore jruby result, probably all passed
And this "error" is probably TravisCI issue

Merging next week otherwise I will forget to release (Too lazy to bring up my Mac on weekend...)

Oh, okay! Great! Happy to help!

I'm also pushing some changes to the fog/fog-backblaze which in turn will make it more equal to its sisters fog-aws and fog-azurerm. That library needs some updates too but this PR should already make master work at a basic level with Backblaze B2.

@PikachuEXE
Copy link
Member

I'm also pushing some changes to the fog/fog-backblaze

I am interested to know more

This should add support for Backblaze B2 in a basic level. Initially, 
other files inside the bucket references by Asset Sync might get 
compromised if existing_remote_files is set to 'delete'. I'm also 
submitting an update to the fog-backblaze library to limit call for 
files to an specific folder prefix.
@PikachuEXE PikachuEXE merged commit b9e1a7b into AssetSync:master Dec 14, 2020
@PikachuEXE
Copy link
Member

Released in 2.13.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants