Deploy to Heroku from Circle CI upon updates to a given branch #642

harlantwood · 2015-05-19T01:20:26Z

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 production or deploy)
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.

…therwise

…move Procfile and server.sh

harlantwood · 2015-05-19T01:30:46Z

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.

harlantwood · 2015-05-19T01:34:06Z

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.

davidfurlong · 2015-05-19T09:34:48Z

@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

harlantwood · 2015-05-19T15:22:18Z

Yes, perfect, thanks @davidfurlong

willzeng · 2015-05-19T15:24:32Z

The PAPERTRAIL one shouldn't be necessary IIRC

harlantwood · 2015-05-19T16:49:28Z

@willzeng when you have a chance, try out this PR on your test heroku app...

willzeng · 2015-05-19T18:11:28Z

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?

harlantwood · 2015-05-19T21:51:33Z

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?

harlantwood · 2015-05-20T20:42:48Z

~~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~~

harlantwood · 2015-05-21T06:25:38Z

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?

willzeng · 2015-05-21T10:42:40Z

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.

willzeng · 2015-05-21T11:25:30Z

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.

davidfurlong · 2015-05-21T11:38:04Z

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.

willzeng · 2015-05-21T11:45:22Z

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.

davidfurlong · 2015-05-21T12:02:24Z

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

willzeng · 2015-05-21T12:20:14Z

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 notifications@github.com
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
#642 (comment).

davidfurlong · 2015-05-21T12:38:12Z

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)

willzeng · 2015-05-21T12:58:41Z

Agreed. Have given this its own issue for discussion #644 We'll want to resolve this before merging this PR.

harlantwood · 2015-06-04T22:15:22Z

Hm, do you still want to make this happen @willzeng or @davidfurlong?

davidfurlong · 2015-06-05T14:08:00Z

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

harlantwood · 2015-06-05T15:13:10Z

Awesome! Just checking in.

willzeng · 2015-06-06T13:46:13Z

+1

On Fri, Jun 5, 2015, 16:13 Harlan T Wood notifications@github.com wrote:

Awesome! Just checking in.

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

davidfurlong · 2015-06-07T20:22:33Z

see #647 apologies for the delay

harlantwood · 2015-06-07T21:38:46Z

Awesome :)

harlantwood · 2015-08-15T04:00:38Z

Can this be merged? @davidfurlong @willzeng

harlantwood added 4 commits May 13, 2015 22:57

Run bower as npm post-install task; remove libs first due to errors o…

fa0bdb4

…therwise

move asset compilation to bin/compile; use "npm start" for heroku; re…

3ce0730

…move Procfile and server.sh

deploy from Cirlce CI on updates to "production" branch

c3ff010

Merge remote-tracking branch 'canon/dev' into deploy3

bef90bf

harlantwood mentioned this pull request May 19, 2015

Simplify server config; compile prestart #639

Closed

harlantwood mentioned this pull request May 19, 2015

setup gulp -- build script #380

Closed

update to coffeescript ~1.9.2

3874642

harlantwood force-pushed the deploy3 branch from 39234a7 to 3874642 Compare May 21, 2015 06:18

harlantwood mentioned this pull request May 21, 2015

DB credentials checked into codebase #643

Open

willzeng mentioned this pull request May 21, 2015

Refactor stylesheets to be appropriate for build #644

Open

Merge remote-tracking branch 'canon/dev' into deploy3

3a41134

harlantwood mentioned this pull request May 22, 2015

Begin to move DB credentials out of codebase #646

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deploy to Heroku from Circle CI upon updates to a given branch #642

Deploy to Heroku from Circle CI upon updates to a given branch #642

harlantwood commented May 19, 2015

harlantwood commented May 19, 2015

harlantwood commented May 19, 2015

davidfurlong commented May 19, 2015

harlantwood commented May 19, 2015

willzeng commented May 19, 2015

harlantwood commented May 19, 2015

willzeng commented May 19, 2015

harlantwood commented May 19, 2015

harlantwood commented May 20, 2015

harlantwood commented May 21, 2015

willzeng commented May 21, 2015

willzeng commented May 21, 2015

davidfurlong commented May 21, 2015

willzeng commented May 21, 2015

davidfurlong commented May 21, 2015

willzeng commented May 21, 2015

davidfurlong commented May 21, 2015

willzeng commented May 21, 2015

harlantwood commented Jun 4, 2015

davidfurlong commented Jun 5, 2015

harlantwood commented Jun 5, 2015

willzeng commented Jun 6, 2015

davidfurlong commented Jun 7, 2015

harlantwood commented Jun 7, 2015

harlantwood commented Aug 15, 2015

Deploy to Heroku from Circle CI upon updates to a given branch #642

Are you sure you want to change the base?

Deploy to Heroku from Circle CI upon updates to a given branch #642

Conversation

harlantwood commented May 19, 2015

harlantwood commented May 19, 2015

harlantwood commented May 19, 2015

davidfurlong commented May 19, 2015

harlantwood commented May 19, 2015

willzeng commented May 19, 2015

harlantwood commented May 19, 2015

willzeng commented May 19, 2015

harlantwood commented May 19, 2015

harlantwood commented May 20, 2015

harlantwood commented May 21, 2015

willzeng commented May 21, 2015

willzeng commented May 21, 2015

davidfurlong commented May 21, 2015

willzeng commented May 21, 2015

davidfurlong commented May 21, 2015

willzeng commented May 21, 2015

davidfurlong commented May 21, 2015

willzeng commented May 21, 2015

harlantwood commented Jun 4, 2015

davidfurlong commented Jun 5, 2015

harlantwood commented Jun 5, 2015

willzeng commented Jun 6, 2015

davidfurlong commented Jun 7, 2015

harlantwood commented Jun 7, 2015

harlantwood commented Aug 15, 2015