Fix - Django functions with add_converter#951
Conversation
✅ Deploy Preview for dynaconf ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## master #951 +/- ##
==========================================
- Coverage 99.64% 99.64% -0.01%
==========================================
Files 22 22
Lines 1994 1990 -4
==========================================
- Hits 1987 1983 -4
Misses 7 7
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| def django_ready(): | ||
| from django.urls import reverse_lazy # noqa | ||
|
|
||
| dynaconf.add_converter("reverse_lazy", reverse_lazy) | ||
|
|
||
|
|
||
| dynaconf.django_ready = django_ready |
There was a problem hiding this comment.
We can use a decorator to register this kind of hook and allow it to be a sequence?
This will require the django_ready to be a decorator that registers the functions in a list of hooks like.
dynaconf/__init__.py
from functools import wraps
# Key can optionally be an enum here
HOOKS = {
"django_ready": [],
...
}
def instance_hook(identifier):
@wraps
def decorator(f):
HOOKS[identifier].append(f)
return decoratorThen in the execute_loaders
for _hook in dynaconf.HOOKS["django_ready"]:
_hook()
# notes
# we may need to avoid executing the same hook twice
# key can be enum, hook can be a better structured class instance if needed
# I am ok on using strings and just assuming callables.Usage:
import dynaconf
@dynaconf.instance_hook("django_ready")
def add_reverse_url_converter():
from django.urls import reverse_lazy # noqa
dynaconf.add_converter("reverse_lazy", reverse_lazy)|
@rochacbruno Yesterday I've experimented a bit with django translation system and I come up with some conclusions. GettextThe django gettext issue #649 is more complex than I thought and won't (don't have to) work with the same solution provided to fix the CLI, so I'll leave it out of this PR. Just as a note, I've found that from __future__ import annotations
from django.apps import AppConfig
class PollsConfig(AppConfig):
name = "foo"
def ready(self):
from django.utils.translation import gettext_lazy as _
from django.conf import settings
settings.TRANSLATABLE=_("hello world")CLI fix designFor the actual django fix, I've changed my mind about the design.
These are the design, summarized: # 1) existing hook solution (at dynaconf_hooks.py)
def post():
...
# 2) add_hook utility for existing loader hooks
def setup_function():
...
dynaconf.add_hook(setup_function)
# 3) new keyword in DynaconfDjango (and Setting, consequently)
def setup_function():
...
settings = DynaconfDjango(
__name__,
post_hook=setup_function # or [setup_function]
)
# 4) decorator design
@dynaconf.loading_hook
def setup_function():
...
setup_function() # needs to be called or module-collectedWhat do you think about using the existing hook system instead of creating a new one? If you agree, which design do you prefer? (I go with 3) |
|
@pedro-psb I agree with option 3 as it doesn't require adding new concepts. |
|
will the hooked function receive the settings as argument? just like the existing dynaconf_hooks? |
Shortlog of commits since last release:
Bruno Rocha (4):
Ignore docs build without a tag
Cancel any running CI job when a Push is made to an existing PR or branch (dynaconf#952)
Fix dynaconf#959 cli get will exit code 1 in case of KeyError. (dynaconf#960)
add tech preview note to inspect docs (dynaconf#961)
Pedro Pessoa (9):
Docs - Update envvar.md custom token e.g. to use add_converter (dynaconf#941)
Feature - Inspect and CLI (dynaconf#939)
Fix - Template substitution with variable update (dynaconf#944)
Assert dynaconf#658 works (dynaconf#945)
fix infinite recursions in special case of django app dynaconf#867 (dynaconf#947)
Fix - Django functions with `add_converter` (dynaconf#951)
Fix hooks not re-running on reload dynaconf#850 (dynaconf#953)
update vault and redis warning recommendations. fix dynaconf#950 (dynaconf#954)
Fix - Enable merge equal False (dynaconf#957)
Shortlog of commits since last release:
Bruno Rocha (5):
Ignore docs build without a tag
Cancel any running CI job when a Push is made to an existing PR or branch (#952)
Fix #959 cli get will exit code 1 in case of KeyError. (#960)
add tech preview note to inspect docs (#961)
Build docs
Hugo Prudente (1):
Doc advanced usage for cli overrides dynaconf settings fix #967 (#970)
Marian Ganisin (1):
Feat: Support for multidoc yaml files (#825)
Pedro Pessoa (11):
Docs - Update envvar.md custom token e.g. to use add_converter (#941)
Feature - Inspect and CLI (#939)
Fix - Template substitution with variable update (#944)
Assert #658 works (#945)
fix infinite recursions in special case of django app #867 (#947)
Fix - Django functions with `add_converter` (#951)
Fix hooks not re-running on reload #850 (#953)
update vault and redis warning recommendations. fix #950 (#954)
Fix - Enable merge equal False (#957)
CI - Test docker-compose pyyaml issue (#964)
Fix: unexpected _bypass_evaluation in BoxList (#966)
pedro-psb (1):
Release version 3.2.0
Fixes #949 and provides workaround for #648
Overview:
AppRegistryNotReadyexception when registering Django converters (wrap in special hook)Notes
Did not test #648
Although this solution looks a good workaround for the issue, I've not included specific test for it because I'm not familiar with
gettextto think in a good test setup. It should be done later.Workaround alternative
My first attempt to solve the issue was wrapping the import for each function, but the new way seems more user friendly.