Closed potsbo closed 4 years ago
Changes Missing Coverage | Covered Lines | Changed/Added Lines | % | ||
---|---|---|---|---|---|
lib/asset_sync/engine.rb | 0 | 1 | 0.0% | ||
<!-- | Total: | 20 | 21 | 95.24% | --> |
Files with Coverage Reduction | New Missed Lines | % | ||
---|---|---|---|---|
lib/asset_sync/engine.rb | 1 | 0% | ||
<!-- | Total: | 1 | --> |
Totals | |
---|---|
Change from base Build 407: | 1.009% |
Covered Lines: | 393 |
Relevant Lines: | 596 |
I suggest to rename the new config to something like remote_file_list_cache_file_path
cache_file
doesn't explain it should store a file path or what it represents
(file path of a cache file which caches remote file list)
Feel free to suggest other names 😃
Thank you for the quick review!
remote_file_list_cache_file_path
sounds great to me, bit long but I suppose no name can be too verbose in this context.
@PikachuEXE would you take a look?
Squashing into one commit seemed to be the easiest way.
I just made it a single commit. Thanks for the review.
Releasing tomorrow or Friday
2.11.0
released guys
WHY
Using asset_sync for more than 6 years in production in a repo with 800k lines of Rails, we faced a problem with uploading files to s3. It takes more than 5 min to run asset sync, which is too long for us who deploy 30 times a day. After a bit of investigation, we've found that
get_remote_files
is the bottleneck which takes 4 min. As a workaround, we started using a patched version of this gem, whose diff is here. With this patch, our assets_sync finished less than 30s.It basically caches a list of files that already uploaded to remote buckets, so that we don't have to scan the bucket in the next run.
WHAT
After using the patch for more than 1.5 years in production in a big project, I started thinking this is worth feedbacking to the upstream. Maybe I should have created an issue first but I've already got a patch that's working. Creating a Pull Request might be a good idea.
If there is any concern or request to change please let me know.
Tasks left