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 4 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions sanic/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -1452,3 +1452,39 @@ 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.

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

app.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}
app.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
app.update_config(c)"""

self.config.update_config(config)
175 changes: 107 additions & 68 deletions sanic/config.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import os
import types
from os import environ as os_environ
tomaszdrozdz marked this conversation as resolved.
Show resolved Hide resolved
from re import findall as re_findall
from importlib.util import spec_from_file_location, \
module_from_spec

from sanic.exceptions import PyFileError
from sanic.helpers import import_string
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

@tomaszdrozdz tomaszdrozdz Aug 17, 2020

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

@tomaszdrozdz tomaszdrozdz Aug 25, 2020

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


SANIC_PREFIX = "SANIC_"
Expand Down Expand Up @@ -56,76 +58,13 @@ def __getattr__(self, attr):
def __setattr__(self, attr, value):
self[attr] = value

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)
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)

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
"""
module = types.ModuleType("config")
module.__file__ = filename
try:
with open(filename) as config_file:
exec( # nosec
compile(config_file.read(), filename, "exec"),
module.__dict__,
)
except IOError as e:
e.strerror = "Unable to load configuration file (%s)" % e.strerror
raise
except Exception as e:
raise PyFileError(filename) from e

self.from_object(module)
return True

def from_object(self, obj):
"""Update the values from the given object.
Objects are usually either modules or classes.

Just the uppercase variables in that object are stored in the config.
Example usage::

from yourapplication import default_config
app.config.from_object(default_config)

or also:
app.config.from_object('myproject.config.MyConfigClass')

You should not use this function to load the actual configuration but
rather configuration defaults. The actual config should be loaded
with :meth:`from_pyfile` and ideally from a location not within the
package because the package might be installed system wide.

:param obj: an object holding the configuration
"""
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 @@ -140,6 +79,48 @@ def load_environment_vars(self, prefix=SANIC_PREFIX):
self[config_key] = v


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)"""
tomaszdrozdz marked this conversation as resolved.
Show resolved Hide resolved

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)


# Is in Sanic any better place where to keep this ???
tomaszdrozdz marked this conversation as resolved.
Show resolved Hide resolved

def strtobool(val):
"""
This function was borrowed from distutils.utils. While distutils
Expand All @@ -155,3 +136,61 @@ def strtobool(val):
return False
else:
raise ValueError("invalid truth value %r" % (val,))


# Is in Sanic any better place where to keep this ???

def load_module_from_file_location(*args, **kwargs):
ahopkins marked this conversation as resolved.
Show resolved Hide resolved
"""Returns loaded module provided as a file path.

:param args: look for importlib.util.spec_from_file_location parameters specification
:param kwargs: look for importlib.util.spec_from_file_location parameters specification

So for example You can:

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

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."""

# 1) Get location parameter.
if "location" in kwargs:
location = kwargs["location"]
_l = "kwargs"
elif len(args) >= 2:
location = args[1]
_l = "args"
else:
raise Exception("Provided arguments must conform to importlib.util.spec_from_file_location arguments, \
nonetheless location parameter has to be provided.")

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

# 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 Exception("There are no following environment variables: " + ", ".join(not_defined_env_vars))

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

# 3) Put back parsed location pareameter.
if _l == "kwargs":
kwargs["location"] = location
else:
_args = list(args)
_args[1] = location
args = tuple(_args)

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

return module