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

Fixes issues #16, #17, #18 and #19 #20

Merged

Conversation

kranzky
Copy link

@kranzky kranzky commented Nov 11, 2011

These fixes introduce the following changes:

  • manifest was added to configuration to specify the full path to the manifest.yml file generated by the asset pipeline
    • this allows the list of local files to upload to be read from the manifest file
  • if manifest is not given, the list of files is generated by recursing into public/assets as before
    • however, the list of local files is now cached to prevent this from happening more than once
  • the assets sub-directory is now specified when locating files in the S3 bucket, meaning that the bucket may safely contain non-asset files in other sub-directories
  • the full list of asset files already on S3 is now retrieved, rather than the first 1000, preventing unnecessary re-uploads and ensuring all old assets are deleted (if that configuration option was given)
  • the list of files to upload is now correctly calculated:
    • it is now simply the local files minus the remote files; previously it was the xor of the two collections
  • the list of remote files to delete (if that configuration option was specified) is now correctly calculated:
    • it is now simply the remote files minus the local files; previously it was the cor of the two collections
  • new files are uploaded before old files are deleted.

@davidjrice
Copy link
Contributor

This all sounds great

I will be reviewing everything this week.

On 11 Nov 2011, at 01:49, Jason Hutchens wrote:

These fixes introduce the following changes:

  • manifest was added to configuration to specify the full path to the manifest.yml file generated by the asset pipeline
    • this allows the list of local files to upload to be read from the manifest file
  • if manifest is not given, the list of files is generated by recursing into public/assets as before
    • however, the list of local files is now cached to prevent this from happening more than once
  • the assets sub-directory is now specified when locating files in the S3 bucket, meaning that the bucket may safely contain non-asset files in other sub-directories
  • the full list of asset files already on S3 is now retrieved, rather than the first 1000, preventing unnecessary re-uploads and ensuring all old assets are deleted (if that configuration option was given)
  • the list of files to upload is now correctly calculated:
    • it is now simply the local files minus the remote files; previously it was the xor of the two collections
  • the list of remote files to delete (if that configuration option was specified) is now correctly calculated:
    • it is now simply the remote files minus the local files; previously it was the cor of the two collections
  • new files are uploaded before old files are deleted.

You can merge this Pull Request by running:

git pull https://github.com/agworld/asset_sync e26f5ca

Or you can view, comment on it, or merge it online at:

#20

-- Commit Summary --

-- File Changes --

M lib/asset_sync/config.rb (3)
M lib/asset_sync/storage.rb (33)

-- Patch Links --

https://github.com/rumblelabs/asset_sync/pull/20.patch
https://github.com/rumblelabs/asset_sync/pull/20.diff


Reply to this email directly or view it on GitHub:
#20


David Rice
+44 (0)75 905 38303
http://davidjrice.co.uk

davidjrice added a commit that referenced this pull request Nov 14, 2011
@davidjrice davidjrice merged commit b941315 into AssetSync:master Nov 14, 2011
@davidjrice
Copy link
Contributor

I've improved this to detect the configured manifest path from Rails.application.config.assets.manifest if set. So it's only a boolean option on AssetSync. Added some extra logging for visibility as to which process it is using to determine assets to upload. Gave it a test run there and it looks good. Think am ready to cut a new version.

Using: Manifest /app/public/assets/manifest.yml
Uploading: /app/public/assets/application-f8f0ab290a5f852498c17bff3fd4d2d5.css.gz in place of assets/application-f8f0ab290a5f852498c17bff3fd4d2d5.css saving 80.88%
Done.

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.

2 participants