Merge pull request #5 from collijk/config-tweak

Inject required args only into plugins config

autogpt/config/config.py

@@ -4,7 +4,7 @@
 import contextlib
 import os
 import re
-from typing import Dict
+from typing import Dict, Union
 
 import yaml
 from colorama import Fore
@@ -83,11 +83,6 @@ class Config(SystemSettings):
     plugins: list[str]
     authorise_key: str
 
-    # Executed immediately after init by Pydantic
-    def model_post_init(self, **kwargs) -> None:
-        if not self.plugins_config.plugins:
-            self.plugins_config = PluginsConfig.load_config(self)
-
 
 class ConfigBuilder(Configurable[Config]):
     default_plugins_config_file = os.path.join(
@@ -206,21 +201,16 @@ def build_config_from_env(cls) -> Config:
             "chat_messages_enabled": os.getenv("CHAT_MESSAGES_ENABLED") == "True",
         }
 
-        # Converting to a list from comma-separated string
-        disabled_command_categories = os.getenv("DISABLED_COMMAND_CATEGORIES")
-        if disabled_command_categories:
-            config_dict[
-                "disabled_command_categories"
-            ] = disabled_command_categories.split(",")
-
-        # Converting to a list from comma-separated string
-        shell_denylist = os.getenv("SHELL_DENYLIST", os.getenv("DENY_COMMANDS"))
-        if shell_denylist:
-            config_dict["shell_denylist"] = shell_denylist.split(",")
+        config_dict["disabled_command_categories"] = _safe_split(
+            os.getenv("DISABLED_COMMAND_CATEGORIES")
+        )
 
-        shell_allowlist = os.getenv("SHELL_ALLOWLIST", os.getenv("ALLOW_COMMANDS"))
-        if shell_allowlist:
-            config_dict["shell_allowlist"] = shell_allowlist.split(",")
+        config_dict["shell_denylist"] = _safe_split(
+            os.getenv("SHELL_DENYLIST", os.getenv("DENY_COMMANDS"))
+        )
+        config_dict["shell_allowlist"] = _safe_split(
+            os.getenv("SHELL_ALLOWLIST", os.getenv("ALLOW_COMMANDS"))
+        )
 
         config_dict["google_custom_search_engine_id"] = os.getenv(
             "GOOGLE_CUSTOM_SEARCH_ENGINE_ID", os.getenv("CUSTOM_SEARCH_ENGINE_ID")
@@ -230,13 +220,13 @@ def build_config_from_env(cls) -> Config:
             "ELEVENLABS_VOICE_ID", os.getenv("ELEVENLABS_VOICE_1_ID")
         )
 
-        plugins_allowlist = os.getenv("ALLOWLISTED_PLUGINS")
-        if plugins_allowlist:
-            config_dict["plugins_allowlist"] = plugins_allowlist.split(",")
-
-        plugins_denylist = os.getenv("DENYLISTED_PLUGINS")
-        if plugins_denylist:
-            config_dict["plugins_denylist"] = plugins_denylist.split(",")
+        config_dict["plugins_allowlist"] = _safe_split(os.getenv("ALLOWLISTED_PLUGINS"))
+        config_dict["plugins_denylist"] = _safe_split(os.getenv("DENYLISTED_PLUGINS"))
+        config_dict["plugins_config"] = PluginsConfig.load_config(
+            config_dict["plugins_config_file"],
+            config_dict["plugins_denylist"],
+            config_dict["plugins_allowlist"],
+        )
 
         with contextlib.suppress(TypeError):
             config_dict["image_size"] = int(os.getenv("IMAGE_SIZE"))
@@ -318,3 +308,10 @@ def check_openai_api_key(config: Config) -> None:
         else:
             print("Invalid OpenAI API key!")
             exit(1)
+
+
+def _safe_split(s: Union[str, None], sep: str = ",") -> list[str]:
+    """Split a string by a separator. Return an empty list if the string is None."""
+    if s is None:
+        return []
+    return s.split(sep)

autogpt/core/configuration/schema.py

@@ -49,14 +49,7 @@ def build_agent_configuration(cls, configuration: dict) -> S:
         defaults = cls.default_settings.dict()
         final_configuration = deep_update(defaults, configuration)
 
-        config_obj = cls.default_settings.__class__.parse_obj(final_configuration)
-
-        if hasattr(config_obj, "model_post_init"):
-            # HOTFIX: Call model_post_init explictly as it doesn't seem to be called for pydantic<2.0.0
-            # https://github.com/pydantic/pydantic/issues/1729#issuecomment-1300576214
-            config_obj.model_post_init()
-
-        return config_obj
+        return cls.default_settings.__class__.parse_obj(final_configuration)
 
 
 def _get_user_config_fields(instance: BaseModel) -> dict[str, Any]:

autogpt/plugins/plugins_config.py

@@ -1,13 +1,9 @@
 from __future__ import annotations
 
 import os
-from typing import TYPE_CHECKING, Union
+from typing import Union
 
 import yaml
-
-if TYPE_CHECKING:
-    from autogpt.config import Config
-
 from pydantic import BaseModel
 
 from autogpt.logs import logger
@@ -30,11 +26,20 @@ def is_enabled(self, name) -> bool:
         return plugin_config is not None and plugin_config.enabled
 
     @classmethod
-    def load_config(cls, global_config: Config) -> "PluginsConfig":
+    def load_config(
+        cls,
+        plugins_config_file: str,
+        plugins_denylist: list[str],
+        plugins_allowlist: list[str],
+    ) -> "PluginsConfig":
         empty_config = cls(plugins={})
 
         try:
-            config_data = cls.deserialize_config_file(global_config=global_config)
+            config_data = cls.deserialize_config_file(
+                plugins_config_file,
+                plugins_denylist,
+                plugins_allowlist,
+            )
             if type(config_data) != dict:
                 logger.error(
                     f"Expected plugins config to be a dict, got {type(config_data)}, continuing without plugins"
@@ -49,13 +54,21 @@ def load_config(cls, global_config: Config) -> "PluginsConfig":
             return empty_config
 
     @classmethod
-    def deserialize_config_file(cls, global_config: Config) -> dict[str, PluginConfig]:
-        plugins_config_path = global_config.plugins_config_file
-        if not os.path.exists(plugins_config_path):
+    def deserialize_config_file(
+        cls,
+        plugins_config_file: str,
+        plugins_denylist: list[str],
+        plugins_allowlist: list[str],
+    ) -> dict[str, PluginConfig]:
+        if not os.path.exists(plugins_config_file):
             logger.warn("plugins_config.yaml does not exist, creating base config.")
-            cls.create_empty_plugins_config(global_config=global_config)
+            cls.create_empty_plugins_config(
+                plugins_config_file,
+                plugins_denylist,
+                plugins_allowlist,
+            )
 
-        with open(plugins_config_path, "r") as f:
+        with open(plugins_config_file, "r") as f:
             plugins_config = yaml.load(f, Loader=yaml.FullLoader)
 
         plugins = {}
@@ -73,23 +86,27 @@ def deserialize_config_file(cls, global_config: Config) -> dict[str, PluginConfi
         return plugins
 
     @staticmethod
-    def create_empty_plugins_config(global_config: Config):
+    def create_empty_plugins_config(
+        plugins_config_file: str,
+        plugins_denylist: list[str],
+        plugins_allowlist: list[str],
+    ):
         """Create an empty plugins_config.yaml file. Fill it with values from old env variables."""
         base_config = {}
 
-        logger.debug(f"Legacy plugin denylist: {global_config.plugins_denylist}")
-        logger.debug(f"Legacy plugin allowlist: {global_config.plugins_allowlist}")
+        logger.debug(f"Legacy plugin denylist: {plugins_denylist}")
+        logger.debug(f"Legacy plugin allowlist: {plugins_allowlist}")
 
         # Backwards-compatibility shim
-        for plugin_name in global_config.plugins_denylist:
+        for plugin_name in plugins_denylist:
             base_config[plugin_name] = {"enabled": False, "config": {}}
 
-        for plugin_name in global_config.plugins_allowlist:
+        for plugin_name in plugins_allowlist:
             base_config[plugin_name] = {"enabled": True, "config": {}}
 
         logger.debug(f"Constructed base plugins config: {base_config}")
 
-        logger.debug(f"Creating plugin config file {global_config.plugins_config_file}")
-        with open(global_config.plugins_config_file, "w+") as f:
+        logger.debug(f"Creating plugin config file {plugins_config_file}")
+        with open(plugins_config_file, "w+") as f:
             f.write(yaml.dump(base_config))
             return base_config

tests/conftest.py

@@ -59,7 +59,11 @@ def config(
     # avoid circular dependency
     from autogpt.plugins.plugins_config import PluginsConfig
 
-    config.plugins_config = PluginsConfig.load_config(global_config=config)
+    config.plugins_config = PluginsConfig.load_config(
+        plugins_config_file=config.plugins_config_file,
+        plugins_denylist=config.plugins_denylist,
+        plugins_allowlist=config.plugins_allowlist,
+    )
 
     # Do a little setup and teardown since the config object is a singleton
     mocker.patch.multiple(

tests/unit/test_plugins.py

@@ -70,7 +70,11 @@ def test_create_base_config(config: Config):
     config.plugins_denylist = ["c", "d"]
 
     os.remove(config.plugins_config_file)
-    plugins_config = PluginsConfig.load_config(global_config=config)
+    plugins_config = PluginsConfig.load_config(
+        plugins_config_file=config.plugins_config_file,
+        plugins_denylist=config.plugins_denylist,
+        plugins_allowlist=config.plugins_allowlist,
+    )
 
     # Check the structure of the plugins config data
     assert len(plugins_config.plugins) == 4
@@ -102,7 +106,11 @@ def test_load_config(config: Config):
         f.write(yaml.dump(test_config))
 
     # Load the config from disk
-    plugins_config = PluginsConfig.load_config(global_config=config)
+    plugins_config = PluginsConfig.load_config(
+        plugins_config_file=config.plugins_config_file,
+        plugins_denylist=config.plugins_denylist,
+        plugins_allowlist=config.plugins_allowlist,
+    )
 
     # Check that the loaded config is equal to the test config
     assert len(plugins_config.plugins) == 2