1175432 - Use dj-database-url to simplify database environment variables

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect)

Description

[Ed Morley [:emorley]]

We can then replace:

"""
TREEHERDER_DATABASE_NAME = os.environ.get("TREEHERDER_DATABASE_NAME", "treeherder")
TREEHERDER_DATABASE_USER = os.environ.get("TREEHERDER_DATABASE_USER", "treeherder_user")
TREEHERDER_DATABASE_PASSWORD = os.environ.get("TREEHERDER_DATABASE_PASSWORD", "treeherder_pass")
TREEHERDER_DATABASE_HOST = os.environ.get("TREEHERDER_DATABASE_HOST", "localhost")

TREEHERDER_RO_DATABASE_USER = os.environ.get("TREEHERDER_RO_DATABASE_USER", TREEHERDER_DATABASE_USER)
TREEHERDER_RO_DATABASE_PASSWORD = os.environ.get("TREEHERDER_RO_DATABASE_PASSWORD", TREEHERDER_DATABASE_PASSWORD)
TREEHERDER_RO_DATABASE_HOST = os.environ.get("TREEHERDER_RO_DATABASE_HOST", TREEHERDER_DATABASE_HOST)

...

DATABASES = {
    "default": {
        "ENGINE": "django.db.backends.mysql",
        "NAME": TREEHERDER_DATABASE_NAME,
        "USER": TREEHERDER_DATABASE_USER,
        "PASSWORD": TREEHERDER_DATABASE_PASSWORD,
        "HOST": TREEHERDER_DATABASE_HOST,
    },
    "read_only": {
        "ENGINE": "django.db.backends.mysql",
        "NAME": TREEHERDER_DATABASE_NAME,
        "USER": TREEHERDER_RO_DATABASE_USER,
        "PASSWORD": TREEHERDER_RO_DATABASE_PASSWORD,
        "HOST": TREEHERDER_RO_DATABASE_HOST,
    }
}
"""


With:

"""
import dj_database_url
DEFAULT_DATABASE_URL = 'mysql://treeherder_user:treeherder_pass@localhost:3306/treeherder'
DATABASES = {
    'default': dj_database_url.config(default=DEFAULT_DATABASE_URL),
    'read_only': dj_database_url.config(env='DATABASE_URL_RO', default=DEFAULT_DATABASE_URL)
}
"""

We then just set the DATABASE_URL and DATABASE_URL_RO in the environment :-)

In the future, we may even be able to replace this too:

"""
if 'IS_HEROKU' in os.environ:
    ca_path = '/app/deployment/aws/combined-ca-bundle.pem'
    for db_name in DATABASES:
        DATABASES[db_name]['OPTIONS'] = {
            'ssl': {
                'ca': ca_path
            }
        }
"""

...if this PR gets merged:
https://github.com/kennethreitz/dj-database-url/pull/52

Comment 1

[Ed Morley [:emorley]]

There's also:
https://github.com/joke2k/django-environ
https://github.com/julianwachholz/dj-config-url
https://github.com/ghickman/django-cache-url
https://github.com/rdegges/django-heroku-memcacheify

Comment 2

[Ed Morley [:emorley]]

(In reply to Ed Morley [:emorley] from comment #1)
> There's also:
> https://github.com/joke2k/django-environ
> https://github.com/julianwachholz/dj-config-url
> https://github.com/ghickman/django-cache-url
> https://github.com/rdegges/django-heroku-memcacheify

Memcachier recommend using django-heroku-memcacheify, so let's just stick with dj-database-url for now.

Comment 3

[Ed Morley [:emorley]]

Attachment: Use dj-database-url to simplify DB environment variables

Before this is deployed, we'll need to update the stage/prod puppet configs & Heroku settings to add the new environment variable.

Comment 4

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/3330b3c5677e790dfb0372c7b5a2805726093a55
Bug 1175432 - Use dj-database-url to simplify DB environment variables

dj-database-url extracts DB host, port, username, password and database
name from the env variable 'DATABASE_URL' (unless another env variable
name is specified). If the env variable is not defined, it falls back to
the default passed to dj_database_url.config().

This means for Heroku and similar we can replace the multiple DB env
variables with just one URL for default & one for read_only.

This also effectively makes the setting of the read only DB variable
mandatory for stage/production/heroku, since DEFAULT_DATABASE_URL won't
be valid for them - so prevents us inadvertently not using the read only
DB.

Before this is deployed, we'll need to update the stage/prod puppet
configs & Heroku settings to add the new environment variable.

Comment 5

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/9371347bc238dd4c83658fc363949a3c5dce86ff
Revert "Bug 1175432 - Use dj-database-url to simplify DB environment variables"

This reverts commit 3330b3c5677e790dfb0372c7b5a2805726093a55.

Comment 6

[Ed Morley [:emorley]]

So I've reverted this, since it turns out on the admin node we actually use the values from local.py, since of course we can't have both stage+prod env variables floating about, or they'd clash. This causes problems, since dj-database-url intentionally only uses env variables (that's the whole point, 12-factor app principals and all that).

As such, we either need to offload the migrate/other DB-touching manage.py commands to a stage/prod-specific node (eg rabbitmq) [though we'll have to leave collectstatic on the admin node] - or else come up with a way for update.py to load a bunch of stage/prod-specific env variables (eg run a shell script with a bunch of exports before each manage.py command).

Comment 7

[Ed Morley [:emorley]]

Comment on attachment 8631586 [details] [review]
Use dj-database-url to simplify DB environment variables

r? on the update.py change to use the new treeherder-env.sh script :-)

Comment 8

[Kendall Libby [:fubar] (he/him)]

err, I don't see a change to update.py ?

Comment 9

[Ed Morley [:emorley]]

Attachment: Use dj-database-url to simplify DB environment variables (relanding)

Ah sorry the old attachment was pointing at the wrong PR.

Comment 10

[Kendall Libby [:fubar] (he/him)]

Comment on attachment 8634072 [details] [review]
Use dj-database-url to simplify DB environment variables (relanding)

ah, thanks. wasn't sure if I just needed more coffee or not. :-)

looks good to me!

Comment 11

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/1af312e0cc0236134b6d4973eb88e1ef43a9648d
Bug 1175432 - Use dj-database-url to simplify DB environment variables

dj-database-url extracts DB host, port, username, password and database
name from the env variable 'DATABASE_URL' (unless another env variable
name is specified). If the env variable is not defined, it falls back to
the default passed to dj_database_url.config().

This means for Heroku and similar we can replace the multiple DB env
variables with just one URL for default & one for read_only.

This also effectively makes the setting of the read only DB variable
mandatory for stage/production/heroku, since DEFAULT_DATABASE_URL won't
be valid for them - so prevents us inadvertently not using the read only
DB.

The deployment script also had to be updated, so that we set the
prod/stage-specific environment variables before using manage.py, since
dj-database-url cannot rely on what's in the stage/prod local.py config
(which isn't a bad thing, since we're deprecating that file).

Comment 12

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/f92065534af358df75706fd38cd4b6391c19fff6
Bug 1175432 - Make it more obvious if DATABASE_URL is not set

Removes the default value for DATABASE_URL and DATABASE_URL_RO, so that
if either are not set, there is a clearer upfront Django error about
Databases not being configured, rather than timeouts trying to connect
to localhost.

This makes it more obvious when setting up a new instance that the
variables are not present in the environment (and would have avoided
some confusion on Heroku today, when DATABASE_URL was set, but
DATABASE_URL_RO was not) - in exchange for now needing two extra lines
in the puppet config for local testing.

Comment 13

[Ed Morley [:emorley]]

This was fine on deploy to stage and Heroku.