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

Enable concurrent (threaded) uploading of files #284

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sdhull
Copy link

@sdhull sdhull commented Nov 26, 2014

Uploading assets can take more than a few minutes for sufficiently large legacy apps (like the one I work on).

When testing this out locally, I tried with an entirely empty bucket, and our compiled assets is ~40mb. With the threaded uploads, it took about 1 minute. After emptying out the bucket and trying again without threads, it took about 30 minutes.

This can be a bit more taxing on system resources, so I built this feature so that it could be turned off with a config setting.

@kweydt
Copy link

kweydt commented Nov 26, 2014

👍 till the cows come home

@rahilsondhi
Copy link

👍

@sdhull
Copy link
Author

sdhull commented Nov 26, 2014

For what it's worth, it looks like a couple of the configured builds (ruby 1.9.2 with Rails 3.1 and 3.2) are failing to build nokogiri on TravisCI.

Also the specs timeout on the jruby build and I didn't test this with jruby, so I'm really not sure if maybe it behaves strangely with that ruby.

@@ -59,6 +61,7 @@ def initialize
self.enabled = true
self.run_on_precompile = true
self.cdn_distribution_id = nil
self.concurrent_uploads = true
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be off by default?

@scudco
Copy link

scudco commented Nov 26, 2014

It seems to me that all concurrency-related code should be extracted instead of intermingled with conditionals.

@maciejkowalski
Copy link

👍

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.

5 participants