-
-
Notifications
You must be signed in to change notification settings - Fork 61
feat: added support for toml config #88
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
Changes from all 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,12 +1,83 @@ | ||
| import os | ||
| import logging | ||
| from typing import Any, Dict, Union | ||
| from configparser import ConfigParser | ||
|
|
||
| from deprecation import deprecated | ||
| import tomlkit | ||
|
|
||
| from aw_core import dirs | ||
| from aw_core.__about__ import __version__ | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| def _merge(a: dict, b: dict, path=None): | ||
| """ | ||
| Recursively merges b into a, with b taking precedence. | ||
|
|
||
| From: https://stackoverflow.com/a/7205107/965332 | ||
| """ | ||
| if path is None: | ||
| path = [] | ||
| for key in b: | ||
| if key in a: | ||
| if isinstance(a[key], dict) and isinstance(b[key], dict): | ||
| _merge(a[key], b[key], path + [str(key)]) | ||
| elif a[key] == b[key]: | ||
| pass # same leaf value | ||
| else: | ||
| a[key] = b[key] | ||
| else: | ||
| a[key] = b[key] | ||
| return a | ||
|
|
||
|
|
||
| def _comment_out_toml(s: str): | ||
| return "\n".join(["#" + line for line in s.split("\n")]) | ||
|
|
||
|
|
||
| def load_config_toml( | ||
| appname: str, default_config: str | ||
| ) -> Union[dict, tomlkit.container.Container]: | ||
| config_dir = dirs.get_config_dir(appname) | ||
| config_file_path = os.path.join(config_dir, "{}.toml".format(appname)) | ||
|
|
||
| # Run early to ensure input is valid toml before writing | ||
| default_config_toml = tomlkit.parse(default_config) | ||
|
|
||
| # Override defaults from existing config file | ||
| if os.path.isfile(config_file_path): | ||
| with open(config_file_path, "r") as f: | ||
| config = f.read() | ||
| config_toml = tomlkit.parse(config) | ||
| else: | ||
| # If file doesn't exist, write with commented-out default config | ||
| with open(config_file_path, "w") as f: | ||
| f.write(_comment_out_toml(default_config)) | ||
| config_toml = dict() | ||
|
|
||
| config = _merge(default_config_toml, config_toml) | ||
|
|
||
| return config | ||
|
|
||
|
|
||
| def save_config_toml(appname: str, config: str) -> None: | ||
| # Check that passed config string is valid toml | ||
| assert tomlkit.parse(config) | ||
|
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. Should we have a schema to test against here and/or in the tests? Also does that assert result in a good runtime error?
Member
Author
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. Since each user (server, watchers) of the API decides itself what config options to use I'm not sure if a schema makes sense. But maybe it could check against the default config or something to make sure things are of the same type etc? Sounds like a lot of work though, and it will probably work just fine and errors should be rare and clearly user-caused (since the user will know that they edited the config).
Member
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. Would be nice with a schema, but considering the work needed I don't think it's worth it. |
||
|
|
||
| config_dir = dirs.get_config_dir(appname) | ||
| config_file_path = os.path.join(config_dir, "{}.toml".format(appname)) | ||
|
|
||
| with open(config_file_path, "w") as f: | ||
| f.write(config) | ||
|
|
||
|
|
||
| @deprecated( | ||
| details="Use the load_config_toml function instead", | ||
| deprecated_in="0.4.2", | ||
| current_version=__version__, | ||
| ) | ||
| def load_config(appname, default_config): | ||
| """ | ||
| Take the defaults, and if a config file exists, use the settings specified | ||
|
|
@@ -15,7 +86,7 @@ def load_config(appname, default_config): | |
| config = default_config | ||
|
|
||
| config_dir = dirs.get_config_dir(appname) | ||
| config_file_path = os.path.join(config_dir, "{}.ini".format(appname)) | ||
| config_file_path = os.path.join(config_dir, "{}.toml".format(appname)) | ||
|
|
||
| # Override defaults from existing config file | ||
| if os.path.isfile(config_file_path): | ||
|
|
@@ -28,6 +99,11 @@ def load_config(appname, default_config): | |
| return config | ||
|
|
||
|
|
||
| @deprecated( | ||
| details="Use the save_config_toml function instead", | ||
| deprecated_in="0.4.2", | ||
| current_version=__version__, | ||
| ) | ||
| def save_config(appname, config): | ||
| config_dir = dirs.get_config_dir(appname) | ||
| config_file_path = os.path.join(config_dir, "{}.ini".format(appname)) | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
This is the same way that aw-server-rust does it.
Whether it's the best solution though I'm not sure as the default config might become outdated if it gets updated in the future. It's a decent solution anyway. The other possible solutions I have in mind are neither better or worse just different, so might as well just be consistent with how aw-server-rust does it.