Repository: attzonko/mmpy_bot (language: python)

Issue:
2.0.0 release
Let's use this issue to keep track of the to-do list for the upcoming 2.0.0 release. Now that the current version lives on its own branch, we should be able to merge #153 into master and then work from there.

To do:
- [x] Update all documentation
- [x] Let the `Settings` class try to infer values from environment variables if present
- [x] Provide a docker image with an entrypoint that can easily be overridden to add custom plugins etc.
- [ ] Add migration guide to docs
- [ ] Improve logging (it would be nice if the bot facilitates easily logging to file, and the logs should be more extensive with messages of various logging levels)

At release time:
- [x] Publish the docker image as `mmpy_bot:2.0.0`.
- [x] Publish new PyPi package `mmpy_bot:2.0.0`


Discussion / hints:
Agreed, I'll merge #153 with the version as 2.0.0.dev1 and once we finish this todo list we can make it the official 2.0.0 and release to pypi. 
In regards to the documentation, how do we feel update updating README.md to cut out everything south of the "Run the bot" section? I think the information about plugins, attachments and files can be covered in ./docs/.
Replace that with links to specific sections in the docs? Much shorter, same effect.
Yes, good idea. The example plugins are already a showcase of the functionality, so we can just link to those and add a kind of table of contents that links to each of the doc sections :+1:

The "Run the bot" section needs a little update, but I hope to take care of the environment settings and docker image tomorrow and will then update that part.

Fix patch:
diff --git a/entrypoint.py b/entrypoint.py
new file mode 100644
index 00000000..0a0d4862
--- /dev/null
+++ b/entrypoint.py
@@ -0,0 +1,7 @@
+from mmpy_bot import Bot, ExamplePlugin, Settings, WebHookExample
+
+bot = Bot(
+    settings=Settings(),  # Either specify your settings here or as environment variables.
+    plugins=[ExamplePlugin(), WebHookExample()],  # Add your own plugins here.
+)
+bot.run()
diff --git a/mmpy_bot/bot.py b/mmpy_bot/bot.py
index f003f94f..698c8024 100644
--- a/mmpy_bot/bot.py
+++ b/mmpy_bot/bot.py
@@ -1,7 +1,7 @@
 import asyncio
 import logging
 import sys
-from typing import Sequence
+from typing import Optional, Sequence
 
 from mmpy_bot.driver import Driver
 from mmpy_bot.event_handler import EventHandler
@@ -18,24 +18,27 @@ class Bot:
     """
 
     def __init__(
-        self, settings=Settings(), plugins=[ExamplePlugin(), WebHookExample()]
+        self,
+        settings: Optional[Settings] = None,
+        plugins: Sequence[Plugin] = [ExamplePlugin(), WebHookExample()],
     ):
+        # Use default settings if none were specified.
+        self.settings = settings or Settings()
         logging.basicConfig(
             **{
                 "format": "[%(asctime)s] %(message)s",
                 "datefmt": "%m/%d/%Y %H:%M:%S",
-                "level": logging.DEBUG if settings.DEBUG else logging.INFO,
+                "level": logging.DEBUG if self.settings.DEBUG else logging.INFO,
                 "stream": sys.stdout,
             }
         )
-        self.settings = settings
         self.driver = Driver(
             {
-                "url": settings.MATTERMOST_URL,
-                "port": settings.MATTERMOST_PORT,
-                "token": settings.BOT_TOKEN,
-                "scheme": settings.SCHEME,
-                "verify": settings.SSL_VERIFY,
+                "url": self.settings.MATTERMOST_URL,
+                "port": self.settings.MATTERMOST_PORT,
+                "token": self.settings.BOT_TOKEN,
+                "scheme": self.settings.SCHEME,
+                "verify": self.settings.SSL_VERIFY,
             }
         )
         self.driver.login()
diff --git a/mmpy_bot/settings.py b/mmpy_bot/settings.py
index 9f19258a..aa2f31e8 100644
--- a/mmpy_bot/settings.py
+++ b/mmpy_bot/settings.py
@@ -1,13 +1,27 @@
-from dataclasses import dataclass, field
-from typing import Sequence
+import collections
+import os
+from dataclasses import dataclass, field, fields
+from typing import Sequence, get_args, get_origin  # type: ignore
+
+
+def _get_comma_separated_list(string: str, type=str):
+    values = string.split(",")
+    # Convert to the specified type if necessary.
+    if type is not str:
+        values = list([type(value) for value in values])
+    return values
 
 
 @dataclass
 class Settings:
-    """Simple dataclass to hold some values.
+    """Specifies the settings to be used by a chatbot. To run a chatbot, you should
+    either create a custom Settings instance with the appropriate values, or modify them
+    through environment variables.
 
-    Will probably be removed in the future. To run a chatbot, you should always create a
-    custom Settings instance with the appropriate values.
+    The order of priority in which settings are obtained is as follows:
+    1. Environment variables
+    2. Constructor arguments, if specified
+    3. The default value set here.
     """
 
     MATTERMOST_URL: str = "https://chat.com"
@@ -26,7 +40,41 @@ class Settings:
     SCHEME: str = field(init=False)  # Will be taken from the URL. Defaults to https.
 
     def __post_init__(self):
+        # Check if any fields need to be overridden by environment variables
+        self._check_environment_variables()
+
         if "://" in self.MATTERMOST_URL:
             self.SCHEME, self.MATTERMOST_URL = self.MATTERMOST_URL.split("://")
         else:
             self.SCHEME = "https"
+
+    def _check_environment_variables(self):
+        for f in fields(self):
+            if f.name in os.environ:
+                self._set_field(f, os.environ[f.name])
+
+    def _set_field(self, f, value: str):
+        if f.default_factory is list:
+            # Assert that the type spec of this attribute is indeed a sequence
+            if not issubclass(get_origin(f.type), collections.abc.Sequence):
+                raise TypeError(
+                    f"Since attribute {f.name} has `default_factory=list`, it "
+                    "should be specified as a Sequence (or a subclass of it). "
+                    f"The actual type is {f.type}."
+                )
+            if len(get_args(f.type)) == 0:
+                raise TypeError(
+                    f"Attribute {f.name} was specified as a Sequence without "
+                    "specifying what kind of objects it contains."
+                )
+            # Use get_args to find out what kind of sequence it is.
+            value = _get_comma_separated_list(value, type=get_args(f.type)[0])
+        elif f.type in [int, float, str]:  # type: ignore
+            value = f.type(value)
+        else:
+            raise TypeError(
+                f"Attribute {f.name} has type {f.type}, which is not supported."
+                "Consider updating Settings.__post_init__ to support converting"
+                " environment variables to this type."
+            )
+        setattr(self, f.name, value)


Test patch:
diff --git a/tests/unit_tests/settings_test.py b/tests/unit_tests/settings_test.py
new file mode 100644
index 00000000..ac35af11
--- /dev/null
+++ b/tests/unit_tests/settings_test.py
@@ -0,0 +1,31 @@
+import os
+
+from mmpy_bot import Settings
+
+
+def test_constructor():
+    default = Settings()
+    assert default.MATTERMOST_URL == "chat.com"
+    assert default.SCHEME == "https"
+    assert default.IGNORE_USERS == []
+
+    manual = Settings(
+        MATTERMOST_URL="http://website.com", IGNORE_USERS=["me", "someone_else"]
+    )
+    assert manual.MATTERMOST_URL == "website.com"
+    assert manual.SCHEME == "http"
+    assert manual.IGNORE_USERS == ["me", "someone_else"]
+
+    # Test default scheme
+    assert Settings(MATTERMOST_URL="website.com").SCHEME == "https"
+
+
+def test_environment():
+    assert Settings(MATTERMOST_URL="website.com").MATTERMOST_URL == "website.com"
+
+    os.environ["MATTERMOST_URL"] = "test_site.org"
+    os.environ["IGNORE_USERS"] = "me,someone_else"
+    s = Settings(MATTERMOST_URL="website.com")
+    assert s.MATTERMOST_URL == "test_site.org"
+    # The environment variable should be parsed into a list.
+    assert s.IGNORE_USERS == ["me", "someone_else"]