-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: add config sources #20
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,6 +1,9 @@ | ||||||
from collections import defaultdict | ||||||
import os | ||||||
import sys | ||||||
from typing import Any | ||||||
from typing import Callable | ||||||
from typing import DefaultDict | ||||||
from typing import Dict | ||||||
from typing import Generic | ||||||
from typing import Iterator | ||||||
|
@@ -14,6 +17,12 @@ | |||||
import warnings | ||||||
|
||||||
|
||||||
if sys.version_info < (3, 8): | ||||||
from typing_extensions import Literal | ||||||
else: | ||||||
from typing import Literal | ||||||
|
||||||
|
||||||
class NoDefaultType(object): | ||||||
def __str__(self): | ||||||
return "" | ||||||
|
@@ -80,7 +89,7 @@ def __init__( | |||||
self.help_default = help_default | ||||||
|
||||||
def _retrieve(self, env, prefix): | ||||||
# type: (Env, str) -> T | ||||||
# type: (Env, str) -> Tuple[ConfigSourceType, T] | ||||||
source = env.source | ||||||
|
||||||
full_name = prefix + _normalized(self.name) | ||||||
|
@@ -114,7 +123,7 @@ def _retrieve(self, env, prefix): | |||||
|
||||||
if raw is None: | ||||||
if not isinstance(self.default, NoDefaultType): | ||||||
return self.default | ||||||
return "default", self.default | ||||||
|
||||||
raise KeyError( | ||||||
"Mandatory environment variable {} is not set".format(full_name) | ||||||
|
@@ -128,37 +137,37 @@ def _retrieve(self, env, prefix): | |||||
type(parsed), self.type | ||||||
) | ||||||
) | ||||||
return parsed | ||||||
return "environment", parsed # type: ignore[return-value] | ||||||
|
||||||
if self.type is bool: | ||||||
return cast(T, raw.lower() in env.__truthy__) | ||||||
return "environment", cast(T, raw.lower() in env.__truthy__) | ||||||
elif self.type in (list, tuple, set): | ||||||
collection = raw.split(env.__item_separator__) | ||||||
return cast(T, self.type(collection if self.map is None else map(self.map, collection))) # type: ignore[call-arg,arg-type,operator] | ||||||
return "environment", cast(T, self.type(collection if self.map is None else map(self.map, collection))) # type: ignore[call-arg,arg-type,operator] | ||||||
elif self.type is dict: | ||||||
d = dict( | ||||||
_.split(env.__value_separator__, 1) | ||||||
for _ in raw.split(env.__item_separator__) | ||||||
) | ||||||
if self.map is not None: | ||||||
d = dict(self.map(*_) for _ in d.items()) | ||||||
return cast(T, d) | ||||||
return "environment", cast(T, d) | ||||||
|
||||||
if _check_type(raw, self.type): | ||||||
return cast(T, raw) | ||||||
return "environment", cast(T, raw) | ||||||
|
||||||
if hasattr(self.type, "__origin__") and self.type.__origin__ is Union: # type: ignore[attr-defined,union-attr] | ||||||
for t in self.type.__args__: # type: ignore[attr-defined,union-attr] | ||||||
try: | ||||||
return cast(T, t(raw)) | ||||||
return "environment", cast(T, t(raw)) | ||||||
except TypeError: | ||||||
pass | ||||||
|
||||||
return self.type(raw) # type: ignore[call-arg,operator] | ||||||
return "environment", self.type(raw) # type: ignore[call-arg,operator,return-type] | ||||||
|
||||||
def __call__(self, env, prefix): | ||||||
# type: (Env, str) -> T | ||||||
value = self._retrieve(env, prefix) | ||||||
# type: (Env, str) -> Tuple[ConfigSourceType, T] | ||||||
source, value = self._retrieve(env, prefix) | ||||||
|
||||||
if self.validator is not None: | ||||||
try: | ||||||
|
@@ -169,7 +178,7 @@ def __call__(self, env, prefix): | |||||
"Invalid value for environment variable %s: %s" % (full_name, e) | ||||||
) | ||||||
|
||||||
return value | ||||||
return source, value | ||||||
|
||||||
|
||||||
class DerivedVariable(Generic[T]): | ||||||
|
@@ -190,6 +199,37 @@ def __call__(self, env): | |||||
return value | ||||||
|
||||||
|
||||||
ConfigSourceType = Literal["default", "environment", "programmatic"] | ||||||
|
||||||
|
||||||
class _EnvSource(object): | ||||||
_sentinel = object() | ||||||
|
||||||
def __init__(self, source_precedence): | ||||||
# type: (List[ConfigSourceType]) -> None | ||||||
self._source_precedence = source_precedence | ||||||
self._sources = {} # type: Dict[ConfigSourceType, Any] | ||||||
for s in self._source_precedence: | ||||||
self._sources[s] = self._sentinel | ||||||
|
||||||
def set_source(self, source, value): | ||||||
# type: (ConfigSourceType, Any) -> None | ||||||
self._sources[source] = value | ||||||
|
||||||
def value(self): | ||||||
for s in self._source_precedence: | ||||||
if self._sources[s] is not self._sentinel: | ||||||
return self._sources[s] | ||||||
return None | ||||||
|
||||||
def value_source_type(self): | ||||||
# type: () -> ConfigSourceType | ||||||
for s in self._source_precedence: | ||||||
if self._sources[s] is not self._sentinel: | ||||||
return s | ||||||
raise ValueError("No source set for setting") | ||||||
|
||||||
|
||||||
class Env(object): | ||||||
"""Env base class. | ||||||
|
||||||
|
@@ -228,7 +268,18 @@ class Env(object): | |||||
|
||||||
def __init__(self, source=None, parent=None): | ||||||
# type: (Optional[Dict[str, str]], Optional[Env]) -> None | ||||||
self.source = source or os.environ | ||||||
# Has to come first to avoid issues with __setattr__ | ||||||
self._items = defaultdict( | ||||||
lambda: _EnvSource( | ||||||
[ | ||||||
"programmatic", | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah I wasn't a fan of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With remote configuration being a thing I opted for |
||||||
"environment", | ||||||
"default", | ||||||
] | ||||||
) | ||||||
) # type: DefaultDict[str, _EnvSource] | ||||||
|
||||||
self.source = source or dict(os.environ) | ||||||
self.parent = parent | ||||||
|
||||||
self._full_prefix = ( | ||||||
|
@@ -243,20 +294,44 @@ def __init__(self, source=None, parent=None): | |||||
derived = [] | ||||||
for name, e in list(self.__class__.__dict__.items()): | ||||||
if isinstance(e, EnvVariable): | ||||||
setattr(self, name, e(self, self._full_prefix)) | ||||||
s, v = e(self, self._full_prefix) | ||||||
self.set_attr_source_value(name, s, v) | ||||||
elif isinstance(e, type) and issubclass(e, Env): | ||||||
if e.__item__ is not None and e.__item__ != name: | ||||||
# Move the subclass to the __item__ attribute | ||||||
setattr(self.spec, e.__item__, e) | ||||||
delattr(self.spec, name) | ||||||
name = e.__item__ | ||||||
setattr(self, name, e(source, self)) | ||||||
setattr(self, name, e(source=self.source, parent=self)) | ||||||
elif isinstance(e, DerivedVariable): | ||||||
derived.append((name, e)) | ||||||
|
||||||
for n, d in derived: | ||||||
setattr(self, n, d(self)) | ||||||
|
||||||
def __setattr__(self, name, value): | ||||||
if name != "_items": | ||||||
self._items[name].set_source("programmatic", value) | ||||||
super(Env, self).__setattr__(name, value) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I love the general idea of keeping track of how a value was set. I just wonder if we could make use of descriptors and dynamic classes here to tidy the logic a bit. For example: from enum import Enum
class Source(Enum):
DEFAULT = 0
OVERRIDE = 1
class VarInstance:
def __init__(self, *args, **kwargs):
try:
super().__init__(*args, **kwargs)
except Exception:
pass
self.source = Source.DEFAULT
# Add support for internal mutation
def append(self, *args, **kwargs):
self.source = Source.OVERRIDE
return super().append(*args, **kwargs)
def make_var_instance(_type):
return type(f"VarInstance[{_type.__name__}]", (VarInstance, _type), {})
# These should be built on demand
IntVarInstance = make_var_instance(int)
ListVarInstance = make_var_instance(list)
# Descriptor wrapper to keep track of the source
class Var:
def __init__(self, value):
self._value = value
self._source = None
def __get__(self, obj, objtype=None):
return self._value
def __set__(self, obj, value):
self._value = IntVarInstance(value)
self._value.source = Source.OVERRIDE
class Config:
var = Var(IntVarInstance(10))
lst = Var(ListVarInstance([1, 2, 3]))
config = Config()
assert config.var == 10, config.var
assert config.var.source == Source.DEFAULT
assert config.lst == [1, 2, 3]
assert config.lst.source == Source.DEFAULT
config.var = 20
config.lst.append(4)
assert config.var == 20
assert config.var.source == Source.OVERRIDE
assert config.lst == [1, 2, 3, 4]
assert config.lst.source == Source.OVERRIDE IIUC, one thing that this change doesn't handle is the case of internal mutation. That is, if a list is mutated programmatically, we don't set the source accordingly (should we?). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @P403n1x87 is this something you would like to do right now or can we leave it for a future refactor? I prefer the simplicity at the moment since we're still fleshing out the feature There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah it would be hard for me to give an "objective" answer here, as TBH I think the descriptor + dynamic types approach is simpler and leaner in my head (and that's why I proposed it). So my honest answer is that I'd prefer we went with this other approach, if we can make it work. Perhaps you might want to get some more eyes on this PR from e.g. the guild so that you could have more opinions besides my very polarised one 🙂 . |
||||||
|
||||||
def set_attr_source_value(self, name, source, value): | ||||||
# type: (str, ConfigSourceType, Any) -> None | ||||||
self._items[name].set_source(source, value) | ||||||
super(Env, self).__setattr__(name, value) | ||||||
|
||||||
def source_type(self, name): | ||||||
# type: (str) -> ConfigSourceType | ||||||
return self._items[name].value_source_type() | ||||||
|
||||||
def __getitem__(self, item): | ||||||
# type: (str) -> Any | ||||||
if item in self._items: | ||||||
return self._items[item].value() | ||||||
raise AttributeError(item) | ||||||
|
||||||
def __contains__(self, item): | ||||||
return item in self._items | ||||||
|
||||||
@classmethod | ||||||
def var( | ||||||
cls, | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we make this an
Enum
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I opted for not since we need to support older Pythons still, is this a blocker @P403n1x87?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 I think I'm getting a bit confused about the aim again. If this is for something that needs to go in ddtrace, it would probably be a new feature, hence it will be first released in a 2.x release. There, we no longer have support for older Pythons. We also don't plan any new 1.x releases. Are we planning to use envier in other projects that still support Python 2?