Merge lp:~facundo/magicicada-server/better-settings-local into lp:magicicada-server

Proposed by Facundo Batista on 2017-05-05
Status: Merged
Approved by: Natalia Bidart on 2017-07-20
Approved revision: 84
Merged at revision: 83
Proposed branch: lp:~facundo/magicicada-server/better-settings-local
Merge into: lp:magicicada-server
Diff against target: 42 lines (+6/-6)
1 file modified
magicicada/settings/__init__.py (+6/-6)
To merge this branch: bzr merge lp:~facundo/magicicada-server/better-settings-local
Reviewer Review Type Date Requested Status
Natalia Bidart 2017-05-05 Approve on 2017-07-16
Review via email: mp+323667@code.launchpad.net

Commit message

Better handling of broken local settings, and allow changing values.

Description of the change

Better handling of broken local settings, and allow changing values.

IOW, I converted the simple "import local and pass if broken" to the following sequence:

- try to import * from local settings

- if broken, inform how!

- if all went ok, also try to import a pre-defined function

- if the function is not there, just pass (it's ok, this new function is optional)

- if the function is there, execute it.

The idea of this new function is to allow the local settings to *modify* a generic settings (not only overwrite it). For an example of how this would be useful, check what I'm doing in my local settings: http://linkode.org/RRo7cFNKgY95YeZyP4b3r6

To post a comment you must log in.
Natalia Bidart (nataliabidart) wrote :

Hum, I really really don't like the fix_settings function. But if you feel very strong about it, could you please at least call it "override_settings"?

For needs like this, what I would propose is using a more "traditional" approach with env vars. Something like:

* in magicicada/settings/__init__.py:

    173 LOGGING = {
    ...
    204 'loggers': {
    205 'django': {
    206 'handlers': ['file'],
    207 'level': os.getenv('MAGICICADA_LOG_LEVEL', 'INFO'),
    208 'propagate': True,
    209 },
    ...
    221 }

And then just export the env var to the value you desire, or execute the server with:

MAGICICADA_LOG_LEVEL=DEBUG make start

Or alternatively, we could define a "debug" policy, a little big more "global", such as this:

LOG_LEVEL = 'INFO' if bool(os.getenv('MAGICICADA_DEBUG', None)) else 'DEBUG'
    173 LOGGING = {
    ...
    204 'loggers': {
    205 'django': {
    206 'handlers': ['file'],
    207 'level': LOG_LEVEL,
    208 'propagate': True,
    209 },
    ...
    221 }

What do you think?

84. By Facundo Batista on 2017-06-28

Better log level setting.

Facundo Batista (facundo) wrote :

Fixed as suggested

Natalia Bidart (nataliabidart) wrote :

Looks good!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'magicicada/settings/__init__.py'
2--- magicicada/settings/__init__.py 2017-04-22 21:07:36 +0000
3+++ magicicada/settings/__init__.py 2017-06-28 01:37:23 +0000
4@@ -1,5 +1,5 @@
5 # Copyright 2008-2015 Canonical
6-# Copyright 2015-2016 Chicharreros (https://launchpad.net/~chicharreros)
7+# Copyright 2015-2017 Chicharreros (https://launchpad.net/~chicharreros)
8 #
9 # This program is free software: you can redistribute it and/or modify
10 # it under the terms of the GNU Affero General Public License as
11@@ -27,7 +27,7 @@
12 https://docs.djangoproject.com/en/1.8/ref/settings/
13 """
14
15-from __future__ import unicode_literals
16+from __future__ import unicode_literals, print_function
17
18 # Build paths inside the project like this: os.path.join(BASE_DIR, ...)
19 import logging
20@@ -204,12 +204,12 @@
21 'loggers': {
22 'django': {
23 'handlers': ['file'],
24- 'level': 'INFO',
25+ 'level': os.getenv('MAGICICADA_LOG_LEVEL', 'INFO'),
26 'propagate': True,
27 },
28 'magicicada.server': {
29 'handlers': ['server'],
30- 'level': 'DEBUG',
31+ 'level': os.getenv('MAGICICADA_LOG_LEVEL', 'DEBUG'),
32 'propagate': False,
33 },
34 'magicicada.metrics': {
35@@ -263,5 +263,5 @@
36
37 try:
38 from magicicada.settings.local import * # noqa
39-except ImportError:
40- pass
41+except ImportError as err:
42+ print("ERROR importing local settings:", err)

Subscribers

People subscribed via source and target branches

to all changes: