Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions halibot/halconfigurer.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,21 +89,26 @@ def validate_config(self, config, fill_default=True):
for key, validator in self.options.items():
tmp = config.get(key)

# Unsure on whether the item this one depends on NOT being present in config is warranted to make this required
required = not validator.depends or validator.depends not in config or config.get(validator.depends)

# Ensure the key exists, and if we aren't taking defaults, ensure we report
if not tmp and not fill_default:
if not tmp and not required:
break
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be a continue instead? if we break we don't validate the rest of the keys

elif not tmp and not fill_default:
missing.append(key)
continue
elif not tmp:
ret[key] = validator.default
continue

# Propogate ValueError if there is one
ret[key] = validator.validate(tmp)
else:
# Propogate ValueError if there is one
ret[key] = validator.validate(tmp)

if missing and not fill_default:
str = "Missing key" + ("s" if len(missing) > 1 else "") + ": " + ", ".join(missing)
raise KeyError(str)

return ret

# Validate an individual key/value pair. Probably used for runtime changes/configurerers.
def validate_key(self, key, value):
validator = self.options.get(key)
Expand Down
11 changes: 10 additions & 1 deletion halibot/halibot.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from .halmodule import HalModule
from .halagent import HalAgent
from .halauth import HalAuth
from .halconfigurer import HalConfigurer

# Avoid appending "." if it i
if "." not in sys.path:
Expand Down Expand Up @@ -120,7 +121,11 @@ def load_object(self, pkg, conf={}):
if len(split) == 2:
obj = self._get_class_from_package(*split)
if obj and self._check_version(obj):
return obj(self, conf=conf)
# Check config and any missing values
cfgr = obj.Configurer()
conf = cfgr.validate_config(conf)
conf['of'] = pkg
return obj(self, conf=conf)
else:
self.log.error("Invalid class identifier {}, must contain only 1 ':'".format(conf["of"]))
return None
Expand All @@ -132,6 +137,10 @@ def _instantiate_objects(self, key):
conf = inst[k]
obj = self.load_object(conf["of"], conf=conf)

if obj.config != conf:
self.config[key + "-instances"][k] = obj.config
self.config._write_config()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that I am a fan of this, maybe we should just give a warning instead? I can be convinced otherwise though.

If I am convinced otherwise, we probably should set a bool and do the write after the loop though.


if obj:
self.add_instance(k, obj)

Expand Down