willzeng
Deploy to Heroku from Circle CI upon updates to a given branch
This PR pushes off the app compilation to a CI server, in this case Circle CI (free for open source projects). When you push to a branch of your choice, your code will be deployed to your heroku app. Later, when we add tests to CI, it will only deploy if tests pass.
To use:
- Specify a branch in circle.yml to be built and deployed on every update (eg
productionordeploy) - Visit your Circle settings page, something like: https://circleci.com/gh/[YOUR GITHUB USER]/cograph/edit -- click "Follow" and visit "Heroku Deployment" on the left menu and follow instructions there
- Add an environment variable to Circle CI:
HEROKU_APP, with the value of your Heroku app name to deploy to
Now when you push to your branch, CI will:
- run
bin/compile, precompiling assets - commit these assets to git
- force push this commit to Heroku
Note that this commit of compiled assets only exists on CI and on Heroku, not on a dev machine, and not on github. This keeps git history free of the noise of generated commits.
As an example, here is Circle CI running this code, and deploying any changes to the production branch to my test cograph heroku instance: -- expand the bin/deploy section to see the compilation and deploy to heroku.
Note that this PR obviates #639.
This also adds a bin/deploy script and npm run deploy command, which can be used to deploy from a local box. Note, however that this will add a commit of the generated assets to your current branch, so CI deploys should be preferred IMO.
A few things from this PR should probably land in the README or a deployment page on the wiki.
BTW, although I believe the deploy to heroku is working just fine, there are issues with my test app on Heroku, possibly due to missing config variables. Currently:
2015-05-19T00:34:19.511899+00:00 app[web.1]: /app/node_modules/express.io/node_modules/express/node_modules/connect/lib/proto.js:103
2015-05-19T00:34:19.511906+00:00 app[web.1]: , search = 1 + req.url.indexOf('?')
2015-05-19T00:34:19.511909+00:00 app[web.1]: ^
2015-05-19T00:34:19.511911+00:00 app[web.1]: TypeError: Cannot read property 'indexOf' of undefined
2015-05-19T00:34:19.511913+00:00 app[web.1]: at Function.app.handle (/app/node_modules/express.io/node_modules/express/node_modules/connect/lib/proto.js:103:27)
2015-05-19T00:34:19.511915+00:00 app[web.1]: at app (/app/node_modules/express.io/node_modules/express/node_modules/connect/lib/connect.js:65:37)
2015-05-19T00:34:19.511916+00:00 app[web.1]: at Object.<anonymous> (/app/app.coffee:66:19)
2015-05-19T00:34:19.511918+00:00 app[web.1]: at Object.<anonymous> (/app/app.coffee:97:4)
2015-05-19T00:34:19.511920+00:00 app[web.1]: at Module._compile (module.js:460:26)
2015-05-19T00:34:19.511921+00:00 app[web.1]: at Object.loadFile (/app/node_modules/coffee-script/lib/coffee-script/coffee-script.js:23:19)
2015-05-19T00:34:19.511923+00:00 app[web.1]: at Module.load (module.js:355:32)
2015-05-19T00:34:19.511925+00:00 app[web.1]: at Function.Module._load (module.js:310:12)
2015-05-19T00:34:19.511926+00:00 app[web.1]: at Module.require (module.js:365:17)
2015-05-19T00:34:19.511928+00:00 app[web.1]: at require (module.js:384:17)
@willzeng (or others) can you post a list of the heroku config variable names? I don't need their values, (eg DB credentials), just to know what should be configured.
@willzeng (or others) can you post a list of the heroku config variable names? I don't need their values, (eg DB credentials), just to know what should be configured.
Is this what you need?
GRAPHENEDB_URL MONGOHQ_URL PAPERTRAIL_API_TOKEN REDISCLOUD_URL TWITTER_CB_URL TWITTER_CONSUMER_KEY TWITTER_CONSUMER_SECRET
@willzeng when you have a chance, try out this PR on your test heroku app...
Just as a #TODO note we should probably make it so that if you deploy to heroku without the correct config variables set then it prompts you for them (or at least shows a useful error before crashing).
So I also thought I set up the config variable correctly on the test heroku, but it is crashing with the same error you had before @harlantwood.
May 19 11:04:35 graphdocs-test app/web.1: /app/node_modules/express.io/node_modules/express/node_modules/connect/lib/proto.js:103
May 19 11:04:35 graphdocs-test app/web.1: , search = 1 + req.url.indexOf('?')
May 19 11:04:35 graphdocs-test app/web.1: ^
May 19 11:04:35 graphdocs-test app/web.1: TypeError: Cannot read property 'indexOf' of undefined
What was the change to fix?
Agreed, I like friendly error messages, I'd be happy with just raising the right error, as anyone deploying should be able to check the logs, eg:
GRAPHENEDB_URL = process.env.GRAPHENEDB_URL or throw new Error "Please set environment variable GRAPHENEDB_URL"
I haven't found the fix for the error we both hit yet @willzeng... Not sure where the error is originating from, as I think the coffee line numbers in my stack trace are bogus (likely from the compiled JS). Can you see the source of the error?
~~I did npm shrinkwrap and my deploy / app on heroku works -- see also https://docs.npmjs.com/cli/shrinkwrap~~
~~We could either go this way, or figure out what is different this way, and get more specific in package.json~~
Sweet! I tried upgrading to coffee ~1.9.2 and the errors went away; the app now deploys successfully for me.
I've added this to the pull request.
@willzeng can you confirm that this branch now deploys and the app starts successfully for you, and seems to work well?
App is working locally and starts when depoyed to the test. Right now it seems to fail to find the .css files on the heroku deploy.
GET http://graphdocs-test.herokuapp.com/assets/libs/iconset/flaticon.css
graphdocs-test.herokuapp.com/:1 GET http://graphdocs-test.herokuapp.com/assets/libs/jquery/dist/jquery.min.js
graphdocs-test.herokuapp.com/:1 GET http://graphdocs-test.herokuapp.com/assets/libs/bootstrap/dist/js/bootstrap.min.js
graphdocs-test.herokuapp.com/:1 GET http://graphdocs-test.herokuapp.com/assets/libs/bootstrap/dist/css/bootstrap.min.css
Will take a look at why now.
Right so this was because I forgot to switch over the pages to load from the newly built css and js files. If we do that though, things don't exactly line up so well. Here's the result of trying out main-built.css and main-built.js on index.jade:
Looks like the minifying is affecting the styling somewhere.
Looks like the minifying is effecting the styling somewhere.
I can look at it, but its most likely the ordering of the css rules that it's affecting.. additionally not all pages include all the css files. Ideally we would have separate css files, each minified. Otherwise I can make the different stylesheets compatible with one another, so that minifying and including the lot on each page works.
What would seem to make the most sense is to have one minified .css file for the pages (login, logout, index, signup, userpage etc.) and another minified .css file for the actually application. graph.jade would then load both the pages.css file and the application.css file, ideally. What do you think?
Note that at present I think we just intended the main-built.css to go with graph.jade.
What would seem to make the most sense is to have one minified .css file for the pages (login, logout, index, signup, userpage etc.) and another minified .css file for the actually application. graph.jade would then load both the pages.css file and the application.css file, ideally. What do you think?
Note that at present I think we just intended the main-built.css to go with graph.jade.
Ideally (performance wise) we compile a minified version for each page. S.t. each page only loads 1 css file and only has rules which apply to that page
If you want to do it that way then go ahead. Just having the two files seems like it would make code maintenance easier; you're choice though, so go with what you'd like.
On Thu, May 21, 2015 at 1:02 PM David Furlong [email protected] wrote:
What would seem to make the most sense is to have one minified .css file for the pages (login, logout, index, signup, userpage etc.) and another minified .css file for the actually application. graph.jade would then load both the pages.css file and the application.css file, ideally. What do you think?
Note that at present I think we just intended the main-built.css to go with graph.jade.
Ideally (performance wise) we compile a minified version for each page. S.t. each page only loads 1 css file and only has rules which apply to that page
— Reply to this email directly or view it on GitHub https://github.com/willzeng/cograph/pull/642#issuecomment-104246195.
If you want to do it that way then go ahead. Just having the two files seems like it would make code maintenance easier; you're choice though, so go with what you'd like.
Option 1:
- Resolve conflicts between page specific stylesheets by adding classes if necessary
- Maintain a no conflicts policy between page styles
- Have 2 minified css files
Option 2:
- Keep css files the way that they are
- Have a minified css file which consists of a set css files for each page.
As for performance differences, a) both solutions have the same # of HTTP requests b) Option 2 will have smaller # of rules in minified stylesheets c) In Option 1, stylesheet HTTP request will cache across pages, whilst Option 2 wont d) The cost (in time) of HTTP requests is greater than the css drawing time for small stylesheets
I think Option 1 may be preferable for the size of the stylesheets we currently have (for performance and convenience)
Agreed. Have given this its own issue for discussion https://github.com/willzeng/cograph/issues/644 We'll want to resolve this before merging this PR.
Hm, do you still want to make this happen @willzeng or @davidfurlong? Merge pull request
I'm working on the css bit in a branch, I can try and get it done today
+1
On Fri, Jun 5, 2015, 16:13 Harlan T Wood [email protected] wrote:
Awesome! Just checking in.
— Reply to this email directly or view it on GitHub https://github.com/willzeng/cograph/pull/642#issuecomment-109324010.