codeforamerica / cityvoice

A place-based call-in system for gathering and sharing community feedback
MIT License
47 stars 35 forks source link

Replace Postgres cred ENV vars with example database.yml #135

Closed daguar closed 10 years ago

daguar commented 10 years ago

A few steps to this:

migurski commented 10 years ago

What's the advantage of the database.yml file? The environment var is non-removable, right? If Heroku kills the file, maybe it would be simpler to rely on the environment exclusively for development and testing? We’ve been doing this over in Flask-land on the brigade-alpha and civic-json-worker projects, and there’s a bit of a hump in explaining the use of env, but I do appreciate the close correlation between the dev and deploy environments.

daguar commented 10 years ago

(Pinging @rclosner for his thoughts)

I agree -- I really like 12-factor app approach of storing config in the ENV, particularly when the default deploy environment is Heroku. That is in fact why the CityVoice setup is so ENV-var heavy.

database.yml is the more classic Rails style, harkening back to the days when you'd check your configuration for all environments into source control, and Rails would know to case-switch over what environment it was operating in. This is why the file includes config sections for development/test/production. (And this approach remains for more traditional deployment environments, like an Ubuntu box with Capistrano.)

Again, I think this is a pretty minor thing, but here are a few options to choose from:

  1. Stay the same: requiring POSTGRES_USERNAME and POSTGRES_PASSWORD env vars. We're presuming here that they're running Postgres on localhost, but I like that. I think this is the most user-friendly approach.
  2. Require a DATABASE_URL env var like postgres://username:password@localhost/cityvoice_development (Rails automatically picks this up and uses it if it's present.) I personally think this is less user-friendly, even if it's only a single env var instead of two.
  3. Just tell people to configure database.yml for their environment, and provide an example (this is a reasonable and standard approach in Rails apps)

I vote (1), but really don't care that much.

Regardless, using a .env file with dot-env makes the experience of setting all these ENV vars better. @waltz likes this, I know. Probably worth adding an issue to do that, but -- again -- I think a lot this is a lot of cycles for a minor thing.

waltz commented 10 years ago

I like to think of the database.yml file as a place to store application-specific database configuration settings like the connector type, timeout length or encoding. I feel like it'd be overbearing to ask people to put all that into their environment.

I like suggestion #1 on this list here. I've tended to use a pattern where I'll namespace the setting by prefixing it with the name of the thing. (aka what everyone seems to do)

I've shyed away from using the dotenv loader recently. When I started using it there was some really strange use case that I don't really remember and it always felt really funky to me. OTOH, sometimes people really don't understand environment variables and  having the app do some of the lifting can make installation a little easier.

On February 20, 2014 at 9:47:05 AM, Dave Guarino (notifications@github.com) wrote:

(Pinging @rclosner for his thoughts)

I agree -- I really like 12-factor app approach of storing config in the ENV, particularly when the default deploy environment is Heroku. That is in fact why the CityVoice setup is so ENV-var heavy.

database.yml is the more classic Rails style, harkening back to the days when you'd check your configuration for all environments into source control, and Rails would know to case-switch over what environment it was operating in. This is why the file includes config sections for development/test/production. (And this approach remains for more traditional deployment environments, like an Ubuntu box with Capistrano.)

Again, I think this is a pretty minor thing, but here are a few options to choose from:

Stay the same: requiring POSTGRES_USERNAME and POSTGRES_PASSWORD env vars. We're presuming here that they're running Postgres on localhost, but I like that. I think this is the most user-friendly approach. Require a DATABASE_URL env var like postgres://username:password@localhost/cityvoice_development (Rails automatically picks this up and uses it if it's present.) I personally think this is less user-friendly, even if it's only a single env var instead of two. Just tell people to configure database.yml for their environment, and provide an example (this is a reasonable and standard approach in Rails apps) I vote (1), but really don't care that much.

Regardless, using a .env file with dot-env makes the experience of setting all these ENV vars better. @waltz likes this, I know. Probably worth adding an issue to do that, but -- again -- I think a lot this is a lot of cycles for a minor thing.

— Reply to this email directly or view it on GitHub.

migurski commented 10 years ago

I definitely agree that keeping DB connection config stuff like timeouts or encoding out of the environment vars makes sense.

I’m still not confident that the username/password approach (1) is quite right—it works great if you’re on localhost and the default port, screws you if you're not, and is (I think) redundant with the existing and necessary Heroku environment var. I don’t have strong feelings about this, just a bit uncomfortable with the same resource being configured in two different ways during the same install. So, I guess I'm more in support of suggestion (2). It’s less friendly due to the perceived complexity of the configuration var but perhaps more forgiving of confusion by remaining self-consistent? Those URLs are indeed ugly.

migurski commented 10 years ago

Also, FWIW: the database URL approach leaves open the possibility of a database that isn’t Postgres at all. I’m working on another repo with an ORM right now (Flask app with SQLAlchemy) and grateful that I can work in Postgres on Heroku and SQLite locally.

daguar commented 10 years ago

So I think we're starting to bikeshed here.

I looked at the 6 most-starred repos at www.OpenSourceRails.com and of the ones with ActiveRecord configuration, they're mostly using example database.ymls and assuming people will change them for their own setups:

Project Approach Link
GitLab provides example database.yml for Postgres and MySQL https://github.com/gitlabhq/gitlabhq/tree/master/config
Diaspora provides example database.yml https://github.com/diaspora/diaspora/blob/develop/config/database.yml.example
Self-Starter provides default database.yml using SQLite https://github.com/lockitron/selfstarter/blob/master/config/database.yml

So I vote to keep the default POSTGRES_USERNAME and POSTGRES_PASSWORD approach, noting that you can alternatively edit database.yml to customize your database connection.

Savvy Rails folks will know they can override this with DATABASE_URL in env, and since I think being savvy is a pre-req for constructing that in the first place, I don't think it's worth mentioning.

Kosher?

migurski commented 10 years ago

Kosher!

daguar commented 10 years ago

Closing with https://github.com/codeforamerica/cityvoice/commit/65fe2e0143f08d282a2b2927dcf6eff85bea59b2