-
Notifications
You must be signed in to change notification settings - Fork 910
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
Add globals feature for OmegaConfigLoader
using a globals resolver
#2921
Changes from 13 commits
3e2e73e
9223f85
44bc9e7
f4ffa30
5648999
ced46c5
ee285f4
7221a16
6ad693f
e49f72f
4fd5da0
84bf3d1
bd84d0a
b004b87
c099422
6cef54b
0d5d95d
d159cdf
78793ef
17789a7
b8b066d
d76c022
6e9c8b0
bed4106
4b1b6f4
01af470
92ab551
ed91395
ca622b7
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 | ||||
---|---|---|---|---|---|---|
|
@@ -11,6 +11,7 @@ | |||||
|
||||||
import fsspec | ||||||
from omegaconf import OmegaConf | ||||||
from omegaconf.errors import InterpolationResolutionError | ||||||
from omegaconf.resolvers import oc | ||||||
from yaml.parser import ParserError | ||||||
from yaml.scanner import ScannerError | ||||||
|
@@ -109,6 +110,7 @@ def __init__( # noqa: too-many-arguments | |||||
"parameters": ["parameters*", "parameters*/**", "**/parameters*"], | ||||||
"credentials": ["credentials*", "credentials*/**", "**/credentials*"], | ||||||
"logging": ["logging*", "logging*/**", "**/logging*"], | ||||||
"globals": ["globals*", "globals*/**", "**/globals*"], | ||||||
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
I suggest keep this pattern simple.
I may even suggest a more conservative default WDYT? @ankatiyar @merelcht , cc @stichbury because I really think we should separate the terminology for 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. Offline conversation - agree with using default with |
||||||
} | ||||||
self.config_patterns.update(config_patterns or {}) | ||||||
|
||||||
|
@@ -117,7 +119,8 @@ def __init__( # noqa: too-many-arguments | |||||
# Register user provided custom resolvers | ||||||
if custom_resolvers: | ||||||
self._register_new_resolvers(custom_resolvers) | ||||||
|
||||||
# Register globals resolver | ||||||
self._register_globals_resolver() | ||||||
file_mimetype, _ = mimetypes.guess_type(conf_source) | ||||||
if file_mimetype == "application/x-tar": | ||||||
self._protocol = "tar" | ||||||
|
@@ -199,7 +202,7 @@ def __getitem__(self, key) -> dict[str, Any]: | |||||
|
||||||
config.update(env_config) | ||||||
|
||||||
if not processed_files: | ||||||
if not processed_files and key != "globals": | ||||||
raise MissingConfigException( | ||||||
f"No files of YAML or JSON format found in {base_path} or {env_path} matching" | ||||||
f" the glob pattern(s): {[*self.config_patterns[key]]}" | ||||||
|
@@ -308,6 +311,24 @@ def _is_valid_config_path(self, path): | |||||
".json", | ||||||
] | ||||||
|
||||||
def _register_globals_resolver(self): | ||||||
"""Register the globals resolver""" | ||||||
OmegaConf.register_new_resolver( | ||||||
"globals", lambda x: self._get_globals_value(x), replace=True | ||||||
) | ||||||
|
||||||
def _get_globals_value(self, variable): | ||||||
"""Return the globals values to the resolver""" | ||||||
keys = variable.split(".") | ||||||
value = self["globals"] | ||||||
for k in keys: | ||||||
value = value.get(k) | ||||||
if not value: | ||||||
raise InterpolationResolutionError( | ||||||
f"Globals key '{variable}' not found." | ||||||
) | ||||||
return value | ||||||
|
||||||
@staticmethod | ||||||
def _register_new_resolvers(resolvers: dict[str, Callable]): | ||||||
"""Register custom resolvers""" | ||||||
|
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.
Note to self: Change version depending on when this is merged.