some polishing

brian_polling_manager/error_report/cli.py

@@ -206,6 +206,15 @@ def get_error_points(client: InfluxDBClient, time_window: str):
 
 
 def select_error_fields(errors, mapping):
+    """Create a dictionary with every target key from `mapping`_ and its corresponding
+    value from the `errors`_ dictionary, or ``0`` if it doesn't exist or has a ``None``
+    value
+
+    :param errors: An error point dictionary coming from Influx
+    :param mapping: A field name mapping {source: target} (ie. `ERROR_FIELDS`) for
+        translating field names from the influx query to their error names in the report
+    :returns: A new dictionary containing all relevant error counts
+    """
     # the extra `or 0` is for substituting None values
     return {tgt: errors.get(src, 0) or 0 for src, tgt in mapping.items()}
 
@@ -295,12 +304,12 @@ def is_excluded_interface(description: str, exclusions: Sequence[str]):
 
 def get_relevant_interfaces(hosts):
     """Get interface info from inventory provider. Some interfaces are considered
-    irrelevant based on there description"""
+    irrelevant based on their description"""
 
-    return _filter_and_convert_interfaces(load_interfaces(hosts))
+    return _filter_and_sort_interfaces(load_interfaces(hosts))
 
 
-def _filter_and_convert_interfaces(interfaces):
+def _filter_and_sort_interfaces(interfaces):
     # We may want to put this logic inside inventory provider and serve from a new
     # endpoint
     return dict(

brian_polling_manager/error_report/config-example.json

@@ -6,7 +6,8 @@
     "cc": "some-cc",
     "hostname": "some.smtp.server",
     "username": "smtp-user",
-    "password": "smtp-password"
+    "password": "smtp-password",
+    "starttls": false
   },
   "inventory": ["blah"],
   "influx": {

brian_polling_manager/error_report/report.py

@@ -8,7 +8,7 @@ import base64
 THIS_DIR = pathlib.Path(__file__).parent
 logger = logging.getLogger(__name__)
 
-SMTP_TIMEOUT = 5  # s
+SMTP_TIMEOUT_SECONDS = 5
 
 
 def render_html(errors, date):
@@ -19,7 +19,10 @@ def render_html(errors, date):
     :param template_file: a jinja2 template to render
     """
     env = jinja2.Environment(
-        loader=jinja2.FileSystemLoader(THIS_DIR), newline_sequence="\r\n"
+        # use CRLF since that was (explicitly) used by the original bash script, perhaps
+        # it's unnecessary
+        loader=jinja2.FileSystemLoader(THIS_DIR),
+        newline_sequence="\r\n",
     )
     template = env.get_template("error_report.html.jinja2")
     return template.render(**errors, date=date)
@@ -39,11 +42,13 @@ def render_email(
 
 
 def send_email(payload: str, config: dict):
-    with smtplib.SMTP(host=config["hostname"], port=25, timeout=SMTP_TIMEOUT) as server:
+    with smtplib.SMTP(
+        host=config["hostname"], port=25, timeout=SMTP_TIMEOUT_SECONDS
+    ) as server:
         if config.get("starttls"):
             server.starttls()
 
-        if config.get("password"):
+        if config.get("password") is not None:
             username = config.get("username", config["from"])
             try:
                 server.login(username, config["password"])

test/error_report/test_error_report.py

@@ -7,7 +7,7 @@ import smtplib
 from unittest.mock import Mock, patch, call
 import brian_polling_manager.error_report.report
 from brian_polling_manager.error_report.report import (
-    SMTP_TIMEOUT,
+    SMTP_TIMEOUT_SECONDS,
     render_email,
     render_html,
     send_email,
@@ -20,7 +20,7 @@ from brian_polling_manager.error_report.cli import (
     INFLUX_TIME_WINDOW_TODAY,
     INFLUX_TIME_WINDOW_YESTERDAY,
     PROCESSED_ERROR_COUNTERS_SCHEMA,
-    _filter_and_convert_interfaces,
+    _filter_and_sort_interfaces,
     get_error_points,
     get_relevant_interfaces,
     interface_errors,
@@ -112,7 +112,7 @@ def create_error_point(mock_influx_client):
 
 @pytest.fixture
 def get_interface_errors(small_inventory, mock_influx_client):
-    interfaces = _filter_and_convert_interfaces(small_inventory)
+    interfaces = _filter_and_sort_interfaces(small_inventory)
 
     def _get(**kwargs):
         defaults = {
@@ -531,7 +531,7 @@ def test_send_email(SMTP):
     }
     send_email("<payload>", config)
     assert SMTP.call_args == call(
-        host=config["hostname"], port=25, timeout=SMTP_TIMEOUT
+        host=config["hostname"], port=25, timeout=SMTP_TIMEOUT_SECONDS
     )
     assert SMTP().__enter__().sendmail.call_args == call(
         config["from"], ["someone.else@geant.org"], "<payload>"
@@ -652,7 +652,7 @@ def test_e2e(
 
         report_b64 = payload.split("\n\n")[-1]
         report = base64.b64decode(report_b64).decode()
-        
+
         assert "mx1.ams.nl.geant.net" in report
         assert "ae1" in report
         assert "input-drops\t1" in report