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

reduce server queries required to trigger recompilation #6

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lazylester
Copy link

git already knows the information so just make a single git query instead of doing a diff for each of the assets directories

git already knows the information so just make a single git query instead of doing a diff for each of the assets directories
@rhomeister
Copy link
Contributor

I'm a little concerned with the scenario in which a previous deploy has failed. What would happen in that case? The new deploy might not have changed any assets, so the assets are not recompiled, even though the previous successfully deployed version has different assets than the current.

@lazylester
Copy link
Author

I think the failed deploy scenario that you describe is covered. Asset precompilation is conditioned on the difference between the current and previous commits, not the current and previous deploys.

If a deploy of commit HEAD fails, then when you re-deploy, you are re-deploying HEAD and deciding whether to precompile based on any asset differences between HEAD~1 and HEAD. So it should be ok.

However, I can now see a scenario where HEAD1 may not have been deployed... the precompile decision must be based on the last commit which was deployed, not the HEAD1 commit. So there could be a problem. Let me look into that scenario.

@rhomeister
Copy link
Contributor

I formulated it a bit clumsily ;), but that's precisely what I meant. Good
luck with your edits. BTW, the delay of querying the server is not very
problematic for me. Do you have a specific problem you wish to solve?

On 22 April 2015 at 03:40, lazylester [email protected] wrote:

I think the failed deploy scenario that you describe is covered. Asset
precompilation is conditioned on the difference between the current and
previous commits, not the current and previous deploys.

If a deploy of commit HEAD fails, then when you re-deploy, you are
re-deploying HEAD and deciding whether to precompile based on any asset
differences between HEAD~1 and HEAD. So it should be ok.

However, I can now see a scenario where HEAD1 may not have been
deployed... the precompile decision must be based on the last commit which
was deployed, not the HEAD
1 commit. So there could be a problem. Let me
look into that scenario.


Reply to this email directly or view it on GitHub
#6 (comment)
.

@lazylester
Copy link
Author

in my rails app, I have a number of engines in vendor/gems, and each has its own app/assets, they are all declared as assets_dependencies. So each requires another query to the server. Life is short. I thought I could see a quicker way.

Looks as if revisions.log has the list of deployed commits. I'm looking at pulling the current & previous deployed commit refs from that file to trigger or inhibit precompilation of assets.

@lazylester
Copy link
Author

@rhomeister I have updated the rake task, now using revisions.log as the reference for the last deployed commit.

This change saves 8 seconds on a deploy with no precompilation (38sec vs 46sec). This is in a rails app with the following declared assets_dependencies:

app/assets lib/assets vendor/assets Gemfile.lock config/routes.rb vendor/gems/authengine/app/assets vendor/gems/authengine/spec/dummy/app/assets vendor/gems/corporate_services/app/assets vendor/gems/nhri/app/assets vendor/gems/outreach_media/app/assets

(the asset path that includes "dummy" is not really needed, it's in the engine's rspec dummy application)

To measure the time I used a shell script:

#!/bin/bash
START=$(date +%s)
cap production deploy
END=$(date +%s)
DIFF=$(( $END - $START ))
echo "It took $DIFF seconds"

I tested scenarios like this:
commit
commit w/asset_change, deploy => recompile triggered
commit w/asset_change, NO deploy
commit w/NO asset change, deploy => recompile triggered
commit w/NO asset change, deploy => no recompile triggered
commit w/asset change, deploy fails, deploy again => recompile triggered

what are your thoughts? no worries if you're concerned about breaking the rake task, but I thought I'd offer the PR anyway. And now it's here, others can see it if they are interested.

cheers

Les

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