#104 WIP: Initial support for using db credentials from OpenShift env
Merged 4 years ago by frantisekz. Opened 4 years ago by frantisekz.

file modified
+16
@@ -4,6 +4,7 @@ 

  import logging

  import logging.handlers

  import os

+ import sys

  

  from .util.login import FakeFas

  
@@ -83,6 +84,21 @@ 

  

  setup_logging()

  

+ # check if we're running in an OpenShift deployment

+ if app.config['OPENSHIFT']:

+     try:

+         dbuser = os.environ["POSTGRESQL_USER"]

+         dbpass = os.environ["POSTGRESQL_PASSWORD"]

+         dbname = os.environ["POSTGRESQL_DATABASE"]

+         dbhost = os.environ["POSTGRESQL_SERVICE_HOST"]

+         dbport = os.environ["POSTGRESQL_SERVICE_PORT"]

+     except(KeyError):

+         print("OpenShift mode enabled but values required for db connection couldn't be fetched. "

+         "Check, if you have these variables defined in you env: "

+         "(POSTGRESQL_[USER, PASSWORD, DATABASE, SERVICE_HOST, SERVICE_PORT])", file=sys.stderr)

+         sys.exit(1)

+     app.config['SQLALCHEMY_DATABASE_URI'] = ("postgresql+psycopg2://%s:%s@%s:%s/%s" % (dbuser, dbpass, dbhost, dbport, dbname))

+ 

  app.logger.debug('using DBURI: %s' % app.config['SQLALCHEMY_DATABASE_URI'])

  

  # database

file modified
+1
@@ -39,6 +39,7 @@ 

      PRODUCTION = False

      FEDMENU_URL = ""

      FEDMENU_DATA_URL = ""

+     OPENSHIFT = False

  

  

  class ProductionConfig(Config):

@@ -15,3 +15,4 @@ 

  STREAM_LOGGING = True

  FEDMENU_URL = "https://apps.fedoraproject.org/fedmenu/"

  FEDMENU_DATA_URL = "https://apps.fedoraproject.org/js/data.js"

+ OPENSHIFT = False

Not yet tested (will do tomorrow)

Make sure to add this also to blockerbugs/config.py

Might be worth either setting some reasonable defaults, or failing (explicitly, instead of dying with a semi-random "could not connect to a db" message) when the env variables are not set.

Might be worth either setting some reasonable defaults, or failing (explicitly, instead of dying with a semi-random "could not connect to a db" message) when the env variables are not set.

Oh, I was (wrongly) assuming it's going to crash on non-existing env var. Thanks!

IIRC getenv('x') returns None when 'x' is not defined. If you wanted to plain fail on KeyError try using os.environ['x']. os.getenv() is basically just a shortcut for os.environ.get()

And even in that case, I'd rather see a try..except block catching the KeyError and dying gracefully with a meaningful error message. Not a hard-requirement, just a nice-to-have.

1 new commit added

  • Changes from PR review
4 years ago

Updated to reflect proposed changes.

2 new commits added

  • Changes from PR review
  • Initial support for using db credentials from OpenShift env
4 years ago

Maybe move this outside of the try block? Not a huge issue, just to have that "critical section" properly isolated.

How about "... but some of the env vars (POSTGRESQL_[USER, PASSWORD, DATABASE, SERVICE_HOST, SERVICE_PORT])...", and also print it out to stderr (not sure that's helpfull or necessary, but I have been in situations where the container just gets killed on exit, and all data is lost).

1 new commit added

  • Moar PR fixes
4 years ago

rebased onto 4c5d199

4 years ago

Pull-Request has been merged by frantisekz

4 years ago