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

Update config #1903

Merged
merged 30 commits into from Sep 30, 2020
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
ae12420
New aproach for uploading sanic app config.
tomaszdrozdz Aug 4, 2020
8a51b97
New aproach for uploading sanic app config.
tomaszdrozdz Aug 4, 2020
84cb859
New aproach for uploading sanic app config.
tomaszdrozdz Aug 4, 2020
87de34d
New aproach for uploading sanic app config.
tomaszdrozdz Aug 4, 2020
3a4c217
Work ongoing. Applying advices from review.
tomaszdrozdz Aug 13, 2020
e81a266
Work ongoing.
tomaszdrozdz Aug 13, 2020
b3baaf0
Work ongoing.
tomaszdrozdz Aug 13, 2020
d79a90c
Work ongoing.
tomaszdrozdz Aug 13, 2020
48b0354
Work ongoing. Aplying advices from Code Review.
tomaszdrozdz Aug 18, 2020
a61641b
Work ongoing. Aplying advices from Code Review.
tomaszdrozdz Aug 18, 2020
2e78087
Work ongoing. Aplying advices from Code Review.
tomaszdrozdz Aug 25, 2020
d5fce53
make fix-import
tomaszdrozdz Aug 25, 2020
9f4fdff
Merge branch 'master' into update_config
ahopkins Aug 25, 2020
cb6a472
Merge branch 'master' into update_config
ahopkins Aug 27, 2020
64387b4
Update config.rst
tomaszdrozdz Aug 27, 2020
8c491c2
Working on documentation.
tomaszdrozdz Aug 27, 2020
3d54b10
Working on documentation.
tomaszdrozdz Aug 28, 2020
f81ee8c
Working on unit tests.
tomaszdrozdz Sep 1, 2020
3f52b13
Working on unit tests.
tomaszdrozdz Sep 1, 2020
00be0b9
Working on unit tests.
tomaszdrozdz Sep 3, 2020
5a4c0bb
Working on unit tests.
tomaszdrozdz Sep 3, 2020
332e207
Working on unit tests.
tomaszdrozdz Sep 4, 2020
0587c94
Work ongoing. Iplementing code reviev advices.
tomaszdrozdz Sep 7, 2020
fd8c34e
Work ongoing. Implementing advices from Code Review.
tomaszdrozdz Sep 8, 2020
b6b54e2
Implementig code reviev advices.
tomaszdrozdz Sep 24, 2020
c2e2eb5
Implementing changes from code review.
tomaszdrozdz Sep 28, 2020
e19b18b
Merge branch 'master' into update_config
ahopkins Sep 30, 2020
4b28e42
Cleanup tests and linting
ahopkins Sep 30, 2020
606e034
squash
ahopkins Sep 30, 2020
3d8bd64
squash
ahopkins Sep 30, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions sanic/app.py
Expand Up @@ -1452,3 +1452,15 @@ async def __call__(self, scope, receive, send):
self.asgi = True
asgi_app = await ASGIApp.create(self, scope, receive, send)
await asgi_app()



# -------------------------------------------------------------------- #
# Configuration
# -------------------------------------------------------------------- #
def update_config(self, config: Union[bytes, str, dict, Any]):
"""Update app.config.

Please refer to config.py: Config class: update_config method for documentation."""

self.config.update_config(config)
99 changes: 79 additions & 20 deletions sanic/config.py
@@ -1,8 +1,19 @@
import os
import types
from os import environ as os_environ
tomaszdrozdz marked this conversation as resolved.
Show resolved Hide resolved

from typing import Union, \
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume that you're using \ charachter here in order to avoid linter errors, am I right? Also, in my opinion would be better to use this format

from typing import Union, Any

# OR

from typing import (Union, Any)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For me it just looks nicer to read.
But if You do not like it - we can do as You like.
Just decide and let me know.

Copy link
Member

Choose a reason for hiding this comment

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

The official stance is that you should run black and isort. We do not need to make these decisions about style choice then 😆

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the black and isort will do the job for us ?

Copy link
Contributor

@myusko myusko Aug 17, 2020

Choose a reason for hiding this comment

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

Yes, actually isort usually doing formatting which I've mentioned above, you need to run black/isort and it will do all necessary formatting, etc.

Copy link
Member

@ahopkins ahopkins Aug 17, 2020

Choose a reason for hiding this comment

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

There should be a command to run this in the Makefile. I usually run make fix-import which (contrary to the name) also runs black. Probably worth a fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So we should not bother much about it because during "building process" it will be "blacked" and "isorted" anyway ?

Copy link
Contributor

Choose a reason for hiding this comment

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

You should manually run make black and make fix-import commands.

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 runned: "make fix-import".
But it changed not only imports - please take a look.
So perhaps fix-import should only fix imports,
and there should be another target that would fix-imports and do other stuff (stuff that now fix-import does).

Any

from .utils import str_to_bool, \
tomaszdrozdz marked this conversation as resolved.
Show resolved Hide resolved
load_module_from_file_location

# \/ \/ \/ \/
# TODO: remove in version: 21.3
tomaszdrozdz marked this conversation as resolved.
Show resolved Hide resolved
import types
from sanic.exceptions import PyFileError
from sanic.helpers import import_string
from warnings import warn
# /\ /\ /\ /\
tomaszdrozdz marked this conversation as resolved.
Show resolved Hide resolved



SANIC_PREFIX = "SANIC_"
Expand Down Expand Up @@ -33,6 +44,7 @@
}



class Config(dict):
def __init__(self, defaults=None, load_env=True, keep_alive=None):
defaults = defaults or {}
Expand All @@ -56,27 +68,42 @@ def __getattr__(self, attr):
def __setattr__(self, attr, value):
self[attr] = value

# \/ \/ \/ \/
# TODO: remove in version: 21.3
def from_envvar(self, variable_name):
tomaszdrozdz marked this conversation as resolved.
Show resolved Hide resolved
"""Load a configuration from an environment variable pointing to
a configuration file.

:param variable_name: name of the environment variable
:return: bool. ``True`` if able to load config, ``False`` otherwise.
"""
config_file = os.environ.get(variable_name)

warn("Using `from_envvar` method is deprecated and will be removed in v21.3, use `app.update_config` method instead.",
DeprecationWarning,
stacklevel=2)

config_file = os_environ.get(variable_name)
if not config_file:
raise RuntimeError(
"The environment variable %r is not set and "
"thus configuration could not be loaded." % variable_name
)
return self.from_pyfile(config_file)
# /\ /\ /\ /\

# \/ \/ \/ \/
# TODO: remove in version: 21.3
def from_pyfile(self, filename):
"""Update the values in the config from a Python file.
Only the uppercase variables in that module are stored in the config.

:param filename: an absolute path to the config file
"""

warn("Using `from_pyfile` method is deprecated and will be removed in v21.3, use `app.update_config` method instead.",
DeprecationWarning,
stacklevel=2)

module = types.ModuleType("config")
module.__file__ = filename
try:
Expand All @@ -93,7 +120,10 @@ def from_pyfile(self, filename):

self.from_object(module)
return True
# /\ /\ /\ /\

# \/ \/ \/ \/
# TODO: remove in version: 21.3
def from_object(self, obj):
"""Update the values from the given object.
Objects are usually either modules or classes.
Expand All @@ -114,18 +144,24 @@ def from_object(self, obj):

:param obj: an object holding the configuration
"""

warn("Using `from_object` method is deprecated and will be removed in v21.3, use `app.update_config` method instead.",
DeprecationWarning,
stacklevel=2)

if isinstance(obj, str):
obj = import_string(obj)
for key in dir(obj):
if key.isupper():
self[key] = getattr(obj, key)
# /\ /\ /\ /\

def load_environment_vars(self, prefix=SANIC_PREFIX):
"""
Looks for prefixed environment variables and applies
them to the configuration if present.
"""
for k, v in os.environ.items():
for k, v in os_environ.items():
if k.startswith(prefix):
_, config_key = k.split(prefix, 1)
try:
Expand All @@ -135,23 +171,46 @@ def load_environment_vars(self, prefix=SANIC_PREFIX):
self[config_key] = float(v)
except ValueError:
try:
self[config_key] = strtobool(v)
self[config_key] = str_to_bool(v)
except ValueError:
self[config_key] = v


def strtobool(val):
"""
This function was borrowed from distutils.utils. While distutils
is part of stdlib, it feels odd to use distutils in main application code.

The function was modified to walk its talk and actually return bool
and not int.
"""
val = val.lower()
if val in ("y", "yes", "t", "true", "on", "1"):
return True
elif val in ("n", "no", "f", "false", "off", "0"):
return False
else:
raise ValueError("invalid truth value %r" % (val,))
def update_config(self, config: Union[bytes, str, dict, Any]):
"""Update app.config.

Note:: only upper case settings are considered.

You can upload app config by providing path to py file holding settings.

# /some/py/file
A = 1
B = 2

config.update_config("${some}/py/file")

Yes you can put environment variable here, but they must be provided in format: ${some_env_var},
and mark that $some_env_var is treated as plain string.

You can upload app config by providing dict holding settings.

d = {"A": 1, "B": 2}
config.update_config(d)

You can upload app config by providing any object holding settings,
but in such case config.__dict__ will be used as dict holding settings.

class C:
A = 1
B = 2
config.update_config(C)"""

if isinstance(config, (bytes, str)):
config = load_module_from_file_location("config", location=config)

if not isinstance(config, dict):
config = config.__dict__

config = dict(filter(lambda i: i[0].isupper(), config.items()))

self.update(config)
5 changes: 5 additions & 0 deletions sanic/exceptions.py
Expand Up @@ -169,6 +169,11 @@ def __init__(self, message, status_code=None, scheme=None, **kwargs):
}


class LoadModuleFromFileLocationException(ImportError):
tomaszdrozdz marked this conversation as resolved.
Show resolved Hide resolved
"""Raised from within utils.py: load_module_from_file_location() function."""
pass


def abort(status_code, message=None):
"""
Raise an exception based on SanicException. Returns the HTTP response
Expand Down
70 changes: 70 additions & 0 deletions sanic/utils.py
@@ -0,0 +1,70 @@
from os import environ as os_environ
from re import findall as re_findall
from importlib.util import spec_from_file_location, \
tomaszdrozdz marked this conversation as resolved.
Show resolved Hide resolved
module_from_spec
from .exceptions import LoadModuleFromFileLocationException
from typing import Union



def str_to_bool(val):
tomaszdrozdz marked this conversation as resolved.
Show resolved Hide resolved
"""Takes string and tries to turn it into bool as human would do.

If val is in case insensitive ("y", "yes", "yep", "yup", "t", "true", "on", "enable", "enabled", "1") returns True.
If val is in case insensitive ("n", "no", "f", "false", "off", "disable", "disabled", "0") returns False.
Else Raise ValueError."""

val = val.lower()
if val in {"y", "yes", "yep", "yup", "t", "true", "on", "enable", "enabled", "1"}:
return True
elif val in {"n", "no", "f", "false", "off", "disable", "disabled", "0"}:
return False
else:
raise ValueError(f"Invalid truth value {val}")



def load_module_from_file_location(name: str, location: Union[bytes, str], enc: str = "utf8", *args, **kwargs):
"""Returns loaded module provided as a file path.

:param name:
The same as importlib.util.spec_from_file_location name param.
:param args:
Coresponds to importlib.util.spec_from_file_location location parameters,
but with this differences:
- It has to be of a string or bytes type.
- You can also use here environment variables in format ${some_env_var}.
Mark that $some_env_var will not be resolved as environment variable.
:enc:
If location parameter is of a bytes type, then use this encoding to decode it into string.
:param args:
Coresponds to the rest of importlib.util.spec_from_file_location parameters.
:param kwargs:
Coresponds to the rest of importlib.util.spec_from_file_location parameters.

For example You can:

some_module = load_module_from_file_location("some_module_name", "/some/path/${some_env_var})"""

# 1) Parse location.
if isinstance(location, bytes):
location = location.decode(enc)

# A) Check if location contains any environment variables in format ${some_env_var}.
env_vars_in_location = set(re_findall("\${(.+?)}", location))

# B) Check these variables exists in environment.
not_defined_env_vars = env_vars_in_location.difference(os_environ.keys())
if not_defined_env_vars:
raise LoadModuleFromFileLocationException(f"The following environment variables are not set: {', '.join(not_defined_env_vars)}")
tomaszdrozdz marked this conversation as resolved.
Show resolved Hide resolved

# C) Substitute them in location.
for env_var in env_vars_in_location:
location = location.replace("${" + env_var + "}", os_environ[env_var])

# 2) Load and return module.
_mod_spec = spec_from_file_location(name, location, *args, **kwargs)
module = module_from_spec(_mod_spec)
_mod_spec.loader.exec_module(module)

return module