Skip to content
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

database config #547

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
/.tool-versions
/node_modules

# Docker Compose override file
*compose.override.y*ml

# Ignore all environment files (except templates).
/.env*
!/.env*.erb
Expand Down
42 changes: 34 additions & 8 deletions config/database.yml
Original file line number Diff line number Diff line change
@@ -1,27 +1,53 @@
<%
# first env with a value
def first_of(*envs)
envs.map { |env| ENV[env] }.compact.first
end
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")
Comment on lines +7 to +19
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

%>

default: &default
adapter: postgresql
# 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" %>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defaults here for when running rails outside of docker

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" %>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

encoding: unicode
pool: <%= ENV.fetch("RAILS_MAX_THREADS") { 5 } %>
port: <%= ENV['HOSTEDGPT_DATABASE_PORT'] || 5432 %>
jlvallelonga marked this conversation as resolved.
Show resolved Hide resolved
<% if RUBY_PLATFORM =~ /darwin/ %>
gssencmode: disable
<% end %>
url: <%= ENV['DATABASE_URL'] %>
# url format: postgres://username:password@localhost:5432/database_name

development:
<<: *default
database: <%= ENV['HOSTEDGPT_DEV_DB'] || "hostedgpt_development" %>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed this in favor of just using the default in development

# It's best to set DATABASE_URL to configure database details. That overrides this "database" name. Note: Docker sets DATABASE_URL inside docker-compose.yml

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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

# It's best to set DATABASE_URL to configure database details. That overrides this "database" name.
database: <%= ENV["HOSTEDGPT_TEST_DB"] || "#{ENV['HOSTEDGPT_DB_NAME'] || 'hostedgpt'}_test" %>

production:
<<: *default
# It's best to set DATABASE_URL to configure database details. That overrides this "database" name.
database: hostedgpt_production
username: hostedgpt
password: <%= ENV["HOSTEDGPT_DATABASE_PASSWORD"] %>
# It's best to set DATABASE_URL to configure database details. That overrides this "database" name.
password: <%= first_of("HOSTEDGPT_DATABASE_PASSWORD", "HOSTEDGPT_DB_PASSWORD") %>
22 changes: 17 additions & 5 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
services:
postgres:
image: postgres:16
container_name: hostedgpt_postgres
restart: always
environment:
POSTGRES_USER: app
POSTGRES_DB: app_development
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rails will create the database so we don't need to make the postgres init process do it

POSTGRES_PASSWORD: secret
# 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
Comment on lines +7 to +9
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

- POSTGRES_USER=${HOSTEDGPT_DB_USERNAME:-hostedgpt_user}
- POSTGRES_PASSWORD=${HOSTEDGPT_DB_PASSWORD:-secret}
volumes:
- hostedgpt_pgdata:/var/lib/postgresql/data
healthcheck:
test: ["CMD", "pg_isready", "-U", "app", "-d", "app_development"]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

test: pg_isready -U $$POSTGRES_USER
interval: 1s
retries: 5

Expand All @@ -22,7 +25,16 @@ services:
target: development
environment:
# Be sure to add environment variables to config/options.yml
- DATABASE_URL=postgres://app:secret@postgres/app_development
- 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
Comment on lines +28 to +33
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

- 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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

- DEV_HOST=${DEV_HOST:-localhost} # Set if you want to use a different hostname
- OVERMIND_COLORS=2,3,5
- ALLOWED_REQUEST_ORIGINS
Expand Down
Loading