diff --git a/brian_polling_manager/error_report/cli.py b/brian_polling_manager/error_report/cli.py index f45becd1bda5829f41e1bd9a51cd5feaba87bb4a..2fa731e9826fe3c78439065ecbd9fdfbf0c25f7e 100644 --- a/brian_polling_manager/error_report/cli.py +++ b/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( diff --git a/brian_polling_manager/error_report/config-example.json b/brian_polling_manager/error_report/config-example.json index 06d6f303fe36cb6c07241e6bb4c4338bb4a0fce3..190d08e6615fb0f2cc6f06884381605654a6d731 100644 --- a/brian_polling_manager/error_report/config-example.json +++ b/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": { diff --git a/brian_polling_manager/error_report/report.py b/brian_polling_manager/error_report/report.py index c2968486d97042b84c05f25ced7bd26e4c60d2fb..3c1764a64498f0912950c7c079be04f241b84e39 100644 --- a/brian_polling_manager/error_report/report.py +++ b/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"]) diff --git a/test/error_report/test_error_report.py b/test/error_report/test_error_report.py index 721f19af7ceb8f9213bc3e7d338d6d5b12313d70..78c4f3390b772f64812f5a3dcadaacb78337a3eb 100644 --- a/test/error_report/test_error_report.py +++ b/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