halibot: add optional config, configfile argument to core#113
Conversation
| if self.use_config: | ||
| self._load_config() | ||
| if not self.config: | ||
| self._load_config(configfile=self.configfile) |
There was a problem hiding this comment.
Forgot to mention this in the description, but I have it here skip loading the config if it was already defined. We may want to change this behavior, but I figure if someone is calling .start(), they can also call ._load_config as well.
There was a problem hiding this comment.
Follow up TODO item: we should really consider config.json validation.
| self.use_auth = kwargs.get("use_auth", True) | ||
| self.workdir = kwargs.get("workdir", ".") | ||
| self.config = kwargs.get("config", {}) | ||
| self.configfile = kwards.get("configfile", "config.json") |
There was a problem hiding this comment.
Typo here, will fix in next edit.
| self.use_config = kwargs.get("use_config", True) | ||
| self.use_auth = kwargs.get("use_auth", True) | ||
| self.workdir = kwargs.get("workdir", ".") | ||
| self.config = kwargs.get("config", {}) |
There was a problem hiding this comment.
Issue here: When we load a config from a file via ._load_config, we update halibot.packages.__path__. If a config is supplied, that step is skipped. Will fix in next edit as well, just need to check if we are supplied a config and update the path accordingly.
update: should be fixed now
With the eventual intent on moving some `main.py` functionality into modules, it will be useful to reuse some of core's logic for temporary cli use. Thus, we will need to spin up temporary instances of `Halibot` to load config, load modules, etc. This adds an optional keyword `config` to `Halibot`, which sets a config dict as supplied. By default, this value is empty, as it was previously. This also adds an optional keyword `configfile` to `Halibot`, to specify an alternate configuration file to load on `._load_config()`. Therefore, this method has also been updated to accept an optional parameter `configfile` parameter of its own, in the event some external code wants to load an alternate code without running the full start process. The default config file remains `config.json`. In theory, no changes to other code is necessary. **NOTE:** With this change, `config` is no longer a static class variable, and is now purely an instance variable. This probably shouldn't affect anything, but multiple `Halibot` objects will no longer have the same config dict object.
7e0afe9 to
ba150da
Compare
sjrct
left a comment
sjrct
left a comment
There was a problem hiding this comment.
There will be some conflicts between this and PR #126, probably easier to merge that before this?
Moving _load_config and _write_config into the Config class defined in that PR as discussed in IRC implies moving configfile into that class as well. I suspect we should make them public methods too,
Should probably also add an argument for specifying the system config path and an argument for ignoring the system config altogether.
| self.workdir = kwargs.get("workdir", ".") | ||
| self.config = kwargs.get("config", {}) | ||
| if self.config: | ||
| halibot.packages.__path__ = self.config.get("package-path", []) |
There was a problem hiding this comment.
With #126 lines 46-47 should not be needed as that assignment is done automatically
I agree, #126 is more interesting than this one. In fact, a lot of this PR could probably be loaded into the
Oh yeah, that.
Agreed. Should also we consider default paths or environment variables for this? |
The default right now is to look alongside the halibot source code, which is inferred from the |
|
Closing this since these functions should be on the |
3 participants
With the eventual intent on moving some
main.pyfunctionality into modules, it will be useful to reuse some of core's logic for temporary cli use. Thus, we will need to spin up temporary instances ofHalibotto load config, load modules, etc.This adds an optional keyword
configtoHalibot, which sets a config dict as supplied. By default, this value is empty, as it was previously.This also adds an optional keyword
configfiletoHalibot, to specify an alternate configuration file to load on._load_config(). Therefore, this method has also been updated to accept an optional parameterconfigfileparameter of its own, in the event some external code wants to load analternate code without running the full start process.
The default config file remains
config.json. In theory, no changes to other code is necessary.NOTE: With this change,
configis no longer a static class variable, and is now purely an instance variable. This probably shouldn't affect anything, but multipleHalibotobjects will no longer have the same config dict object.This should resolve #84