#45 don't ship /etc/waiverdb/settings.py
Merged 6 years ago by mjia. Opened 6 years ago by mjia.
mjia/waiverdb issue41  into  master

file modified
-3
@@ -55,8 +55,6 @@ 

  

  %install

  %py2_install

- install -d %{buildroot}%{_sysconfdir}/waiverdb

- install -p -m 0644 conf/settings.py.example %{buildroot}%{_sysconfdir}/waiverdb/settings.py

  install -d %{buildroot}%{_unitdir}

  install -m0644 \

      systemd/%{name}.service \
@@ -72,7 +70,6 @@ 

  %doc README.md conf docs

  %{python2_sitelib}/%{name}

  %{python2_sitelib}/%{name}*.egg-info

- %{_sysconfdir}/waiverdb

  %{_unitdir}/%{name}.service

  %{_unitdir}/%{name}.socket

  

file modified
+3 -4
@@ -21,7 +21,7 @@ 

  from flask_oidc import OpenIDConnect

  

  

- def load_default_config(app):

+ def load_config(app):

      # Load default config, then override that with a config file

      if os.getenv('DEV') == 'true':

          default_config_obj = 'waiverdb.config.DevelopmentConfig'
@@ -34,8 +34,7 @@ 

          default_config_file = '/etc/waiverdb/settings.py'

      app.config.from_object(default_config_obj)

      config_file = os.environ.get('WAIVERDB_CONFIG', default_config_file)

-     if os.path.exists(config_file):

-         app.config.from_pyfile(config_file)

+     app.config.from_pyfile(config_file)

  

  

  # applicaiton factory http://flask.pocoo.org/docs/0.12/patterns/appfactories/
@@ -44,7 +43,7 @@ 

      if config_obj:

          app.config.from_object(config_obj)

      else:

-         load_default_config(app)

+         load_config(app)

      if app.config['PRODUCTION'] and app.secret_key == 'replace-me-with-something-random':

          raise Warning("You need to change the app.secret_key value for production")

      if app.config['SHOW_DB_URI']:

Since rpmlint is warning about this file, we can stop shipping it. ideally, we will deploy waiverdb into openshift with zero configuration file.

Problem is right now waiverdb won't work without zero configuration. So we would need to fix that before we just drop this from the package entirely. :-)

So we should either change it to be installed into /usr/share/waiverdb/settings.py.example and then make sure we have a startup check to warn the user if they start up without a config -- or just keep /etc/waiverdb/settings.py as is and mark it %config(noreplace) and suppress the bytecode compilation for it.

I don't actually know what the best practice is for config files like this in openshift, but the second option sounds the most straightforward to me:

keep /etc/waiverdb/settings.py as is and mark it %config(noreplace) and suppress the bytecode compilation for it.

^^ that one.

+1 to keeping it and marking it as config.

For OpenShift, I think you'd manage this file as a ConfigMap, and it'd be mounted into /etc/waiverdb/ at runtime:
https://docs.openshift.com/container-platform/3.5/dev_guide/configmaps.html

Right, in OpenShift you replace the whole /etc/waiverdb anyway so the settings.py that ships in the package is just mounted over and never seen.

So I actually think the right answer here is to ship this as an example settings in /usr/share/waiverdb/settings.py.example (and make sure it doesn't get byte compiled either).

+1 to ship this as an example settings in /usr/share/waiverdb/settings.py.example

Actually the settings.py.example file is already shipped in usr/share/doc/waiverdb/conf/settings.py.example. I think the only thing we need to do here is to add a warning
check if users start up wavierdb without a config.

Right now if you start it up (in production mode) without an /etc/waiverdb/settings.py file you get this:

Traceback (most recent call last):
  File "runapp.py", line 19, in <module>
    app = create_app()
  File "/home/dcallagh/work/waiverdb/waiverdb/app.py", line 71, in create_app
    raise Warning("You need to change the app.secret_key value for production")
Warning: You need to change the app.secret_key value for production

but we should be more explicit about what exactly is missing.

I think actually all we need to do is remove the if os.path.exists(config_file): condition from the load_default_config function (which should actually be renamed load_config). Then the error is quite clear:

Traceback (most recent call last):
  File "runapp.py", line 19, in <module>
    app = create_app()
  File "/home/dcallagh/work/waiverdb/waiverdb/app.py", line 68, in create_app
    load_default_config(app)
  File "/home/dcallagh/work/waiverdb/waiverdb/app.py", line 38, in load_default_config
    app.config.from_pyfile(config_file)
  File "/usr/lib/python2.7/site-packages/flask/config.py", line 128, in from_pyfile
    with open(filename) as config_file:
IOError: [Errno 2] Unable to load configuration file (No such file or directory): '/etc/waiverdb/settings.py'

And then we just drop /etc/waiverdb/settings.py from the RPM.

rebased

6 years ago

This is now ready for anther look.

Pull-Request has been merged by mjia

6 years ago
Metadata