-
Notifications
You must be signed in to change notification settings - Fork 209
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
Add deploy to heroku button #36
Comments
+1 This is pretty trivial to do and makes it nice to try things out quickly. I can put together a PR if requested. |
I like the idea and Adding it to the base generated app would be rather simple. I think the tricky bits will be the postgres config. Thinking out loud, since heroku will provide an env var with the full connection string, supporting a url key in the db.json that would get used if exists by connect() would be needed. It would need to take precedence over ie: if (cfg.connectionString) {
connection = anyDB.createPool(cfg.connectionString, {min: 2, max: 20});
} else if (cfg.url && cfg.url.length) {
connection = anyDB.createPool(cfg.url, {min: 2, max: 20});
} else {
connection = anyDB.createPool(
this.adapter.generateConnectionString(cfg.host, cfg.port, cfg.database, cfg.user, cfg.password),
{min: 2, max: 20}
);
} then all that you need to add to the base db.json is "production": {
"main": {
"adapter": "postgres",
"host": "{{= env.DATABASE_HOST }}",
"port": "{{= env.DATABASE_PORT }}",
"user": "{{= env.DATABASE_USER }}",
"password": "{{= env.DATABASE_PASSWORD }}",
"database": "{{= env.DATABASE_DB }}",
"url": "{{= env. DATABASE_URL }}"
} I guess thats 90% of the pr isnt it ;) |
@intabulas : That's actually what @michaelmior Sure, go ahead with the PR. 👍 |
@keithwhor does herkou provide all the connection portions as env vars as well, I thought it did just the url given the examples I looked at. Shows my experience with heroku ;) |
The url they provide includes hostname, username, password, db, table and ssl. |
Okay duh on my part. I was completely thinking of connectionString as an override vs the fact it can be the key in the config... |
No problem! I haven't documented it anywhere and it's not immediately obvious so it's an easy thing to overlook. |
I'd like to consider this in the context of #44 if possible since both affect the DB configuration. I think switching to just using |
Closing, see: #45 We'll add it in the generated README. |
So #46 included a shell of a readme.. We would still need to add app.json and the button in the readme |
https://blog.heroku.com/archives/2014/8/7/heroku-button y/n ?
The text was updated successfully, but these errors were encountered: