database config #547
database config #547
Conversation
| # url format: postgres://username:password@localhost:5432/database_name | ||
| url: <%= ENV["DATABASE_URL"] %> | ||
| # It's best to set DATABASE_URL to configure database details. That overrides the following settings | ||
| username: <%= ENV["HOSTEDGPT_DB_USERNAME"] || "hostedgpt_user" %> |
There was a problem hiding this comment.
defaults here for when running rails outside of docker
| # NOTE: If you use DATABASE_URL, it needs to match these env vars | ||
| # so you can either have it match the default values for these env vars | ||
| # or you can set these env vars to match the user and password from the url |
There was a problem hiding this comment.
this is the issue with using a url and running with docker. If you end up changing the username or password then you have to change it in two places. Pretty minor issue, but still not ideal
| volumes: | ||
| - hostedgpt_pgdata:/var/lib/postgresql/data | ||
| healthcheck: | ||
| test: ["CMD", "pg_isready", "-U", "app", "-d", "app_development"] |
There was a problem hiding this comment.
doesn't seem necessary to have the healthcheck try to connect to the database specifically. Just the database server seems like enough. This way we don't have to put the database name here and have another place to update it
| restart: always | ||
| environment: | ||
| POSTGRES_USER: app | ||
| POSTGRES_DB: app_development |
There was a problem hiding this comment.
rails will create the database so we don't need to make the postgres init process do it
| - HOSTEDGPT_DB_USERNAME=${HOSTEDGPT_DB_USERNAME:-hostedgpt_user} | ||
| - HOSTEDGPT_DB_PASSWORD=${HOSTEDGPT_DB_PASSWORD:-secret} | ||
| - HOSTEDGPT_DB_HOST=${HOSTEDGPT_DB_HOST:-postgres} | ||
| - HOSTEDGPT_DB_PORT=${HOSTEDGPT_DB_PORT:-5432} | ||
| - HOSTEDGPT_DB_NAME=${HOSTEDGPT_DB_NAME:-hostedgpt} | ||
| - DATABASE_URL |
There was a problem hiding this comment.
if DATABASE_URL is set then database.yml will use that as the url option (which will override other settings for the db there), otherwise we use the env vars here (which have default values now)
|
|
||
| development: | ||
| <<: *default | ||
| database: <%= ENV['HOSTEDGPT_DEV_DB'] || "hostedgpt_development" %> |
There was a problem hiding this comment.
removed this in favor of just using the default in development
| def puts_red(msg) | ||
| puts "\e[31m#{msg}\e[0m" | ||
| end | ||
|
|
||
| def deprecated(env, message) | ||
| puts_red "DEPRECATED: #{env} is deprecated, #{message}" if ENV[env] | ||
| end | ||
|
|
||
|
|
||
| deprecated("HOSTEDGPT_DATABASE_PORT", "use HOSTEDGPT_DB_PORT instead") | ||
| deprecated("HOSTEDGPT_DEV_DB", "use HOSTEDGPT_DB_NAME instead") | ||
| deprecated("HOSTEDGPT_TEST_DB", "provide HOSTEDGPT_DB_NAME and test database name is generated") | ||
| deprecated("HOSTEDGPT_DATABASE_PASSWORD", "use HOSTEDGPT_DB_PASSWORD instead") |
There was a problem hiding this comment.
I wish there was a helper file for this. I guess I could put this in lib and include it manually. Maybe this could be an initializer too, but I wanted to keep this close to where the env vars are used. Or if we remove other envs in the future then maybe it's better in an initializer
| password: <%= first_of("HOSTEDGPT_DATABASE_PASSWORD", "HOSTEDGPT_DB_PASSWORD") %> | ||
| host: <%= ENV["HOSTEDGPT_DB_HOST"] %> | ||
| port: <%= first_of("HOSTEDGPT_DATABASE_PORT", "HOSTEDGPT_DB_PORT") || 5432 %> | ||
| database: <%= first_of("HOSTEDGPT_DEV_DB", "HOSTEDGPT_DB_NAME") || "hostedgpt" %> |
There was a problem hiding this comment.
looking for existing env vars first to avoid breaking existing setups. A message is now printed to the console if these deprecated envs are being used.
|
|
||
| test: | ||
| <<: *default | ||
| database: <%= ENV['HOSTEDGPT_TEST_DB'] || "hostedgpt_test" %> | ||
| # regex to match groups in the url: /(before db name)(db name)(after db name)/ | ||
| url: <%= "#{ENV['DATABASE_URL']}".gsub(/(.*\/\/[^\/]+\/)([^?]+)(\??.*)/, '\1\2_test\3') %> # add "_test" to the database name if it's specified |
There was a problem hiding this comment.
we could have them set the test database name manually or set a test database url, but the combinations there get complicated and feels like it adds a lot of work to the user just to allow testing. This way there are less env vars to deal with too
| - HOSTEDGPT_DATABASE_PORT # DEPRECATED: use HOSTEDGPT_DB_PORT | ||
| - HOSTEDGPT_DEV_DB # DEPRECATED: use HOSTEDGPT_DB_NAME | ||
| - HOSTEDGPT_TEST_DB # DEPRECATED: provide HOSTEDGPT_DB_NAME and test database name is generated | ||
| - HOSTEDGPT_DATABASE_PASSWORD # DEPRECATED: use HOSTEDGPT_DB_PASSWORD |
There was a problem hiding this comment.
leaving these in and marking as deprecated because if they're provided then we still want to use them to avoid breaking existing hostedgpt deployments
|
maybe it's extra complexity, but I think this adds some useful functionality and it feels valuable to have consistently named env vars |
found some issues with using a url for the database connection. Hopefully this will fix this for everyone