-
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
Config revamp #82
Config revamp #82
Changes from 8 commits
76d6cb0
ec5f4ec
712a4d7
773d38f
f974c7b
dbb131c
879c411
59ce791
0f6f4a5
b309374
5f048fc
ba1b96a
a0a2b14
3362f97
cdfcda5
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,41 +1,134 @@ | ||
import json | ||
import logging | ||
import copy | ||
|
||
import yaml | ||
|
||
from n2y.utils import strip_hyphens | ||
|
||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
def database_config_json_to_dict(config_json): | ||
DEFAULTS = { | ||
"media_root": "media", | ||
"media_url": "./media/", | ||
} | ||
|
||
|
||
EXPORT_DEFAULTS = { | ||
"id_property": None, | ||
"content_property": None, | ||
"url_property": None, | ||
"notion_filter": [], | ||
"notion_sorts": [], | ||
"pandoc_format": "gfm+tex_math_dollars+raw_attribute", | ||
"pandoc_options": [ | ||
'--wrap', 'none', # don't hard line-wrap | ||
'--eol', 'lf', # use linux-style line endings | ||
], | ||
"plugins": [], | ||
"property_map": {}, | ||
} | ||
|
||
|
||
def load_config(path): | ||
try: | ||
config = json.loads(config_json) | ||
except json.JSONDecodeError as exc: | ||
logger.error("Error parsing the data config JSON: %s", exc.msg) | ||
with open(path, "r") as config_file: | ||
config = yaml.safe_load(config_file) | ||
except yaml.YAMLError as exc: | ||
logger.error("Error parsing the config file: %s", exc) | ||
return None | ||
except FileNotFoundError: | ||
logger.error("The config file '%s' does not exist", path) | ||
return None | ||
if not validate_database_config(config): | ||
if not validate_config(config): | ||
logger.error("Invalid config file: %s", path) | ||
return None | ||
Comment on lines
52
to
63
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. Maybe we can extract this try/except out?
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. Good idea! Done! |
||
|
||
defaults_copy = copy.deepcopy(DEFAULTS) | ||
config = {**defaults_copy, **config} | ||
|
||
merged_exports = merge_config( | ||
config.get("exports", []), | ||
EXPORT_DEFAULTS, | ||
config.get("export_defaults", {}), | ||
) | ||
config["exports"] = merged_exports | ||
return config | ||
|
||
|
||
def validate_database_config(config): | ||
try: | ||
for database_id, config_values in config.items(): | ||
if not _valid_id(database_id): | ||
logger.error("Invalid database id in database config: %s", database_id) | ||
return False | ||
for key, values in config_values.items(): | ||
if key not in ["sorts", "filter"]: | ||
logger.error("Invalid key in database config: %s", key) | ||
return False | ||
if not isinstance(values, dict) and not isinstance(values, list): | ||
logger.error( | ||
"Invalid value of type '%s' for key '%s' in database config, " | ||
"expected dict or list", type(values), key, | ||
) | ||
return False | ||
except AttributeError: | ||
def merge_config(config_items, builtin_defaults, defaults): | ||
""" | ||
For each config item, merge in both the user provided defaults and the | ||
builtin defaults for each key value pair." | ||
""" | ||
merged_config_items = [] | ||
for config_item in config_items: | ||
master_defaults_copy = copy.deepcopy(builtin_defaults) | ||
defaults_copy = copy.deepcopy(defaults) | ||
config_item_copy = copy.deepcopy(config_item) | ||
merged_config_item = {**master_defaults_copy, **defaults_copy, **config_item_copy} | ||
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. Didn't know you could update dictionaries this way. Cool! 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'm sure you looked into this but I assume that with this syntax, the dictionaries get updated in the order they appear right? So it's equivalent to:
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. That's right, the updates apply from left to right! |
||
merged_config_items.append(merged_config_item) | ||
return merged_config_items | ||
|
||
|
||
def validate_config(config): | ||
if "exports" not in config: | ||
logger.error("Config missing the 'exports' key") | ||
return False | ||
if not isinstance(config["exports"], list) and len(config["exports"]) > 0: | ||
logger.error("Config 'exports' key must be a non-empty list") | ||
return False | ||
for export in config["exports"]: | ||
if not _validate_config_item(export): | ||
return False | ||
# TODO: validate the export defaults key | ||
return True | ||
|
||
|
||
def _validate_config_item(config_item): | ||
if "id" not in config_item: | ||
logger.error("Export config item missing the 'id' key") | ||
return False | ||
if not _valid_id(config_item["id"]): | ||
logger.error("Invalid id in export config item: %s", config_item["id"]) | ||
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. Do we need a return False here? 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. Yes! good catch! |
||
if "node_type" not in config_item: | ||
logger.error("Export config item missing the 'node_type' key") | ||
return False | ||
if config_item["node_type"] not in ["page", "database_as_yaml", "database_as_files"]: | ||
logger.error("Invalid node_type in export config item: %s", config_item["node_type"]) | ||
return False | ||
if config_item["node_type"] == "database_as_files" and "filename_property" not in config_item: | ||
logger.error("Missing the 'filename_property' key when node_type is 'database_as_files'") | ||
return False | ||
if "output" not in config_item: | ||
logger.error("Export config item missing the 'output' key") | ||
return False | ||
if "notion_filter" in config_item: | ||
if not _valid_notion_filter(config_item["notion_filter"]): | ||
return False | ||
if "notion_sorts" in config_item: | ||
if not _valid_notion_sort(config_item["notion_sorts"]): | ||
return False | ||
# TODO: validate pandoc_formation | ||
# TODO: validate pandoc_options | ||
# TODO: property map | ||
return True | ||
|
||
|
||
def _valid_notion_filter(notion_filter): | ||
if not (isinstance(notion_filter, list) or isinstance(notion_filter, dict)): | ||
logger.error("notion_filter must be a list or dict") | ||
return False | ||
# TODO validate keys and values | ||
return True | ||
|
||
|
||
def _valid_notion_sort(notion_sorts): | ||
if not (isinstance(notion_sorts, list) or isinstance(notion_sorts, dict)): | ||
logger.error("notion_sorts must be a list or dict") | ||
return False | ||
# TODO validate keys and values | ||
return True | ||
|
||
|
||
|
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.
Is there a reason we're inlining this logic?
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 had thought that, since we know what type of link it is, it would be better to avoid the unnecessary API request that's present if we use
get_page_or_database
.