From 772b004eb3e4108f001ec1595bd845aca5fab7e8 Mon Sep 17 00:00:00 2001 From: Pelle Koster <pelle.koster@geant.org> Date: Wed, 10 Apr 2024 07:34:44 +0200 Subject: [PATCH] some polishing --- brian_polling_manager/error_report/cli.py | 15 ++++++++++++--- .../error_report/config-example.json | 3 ++- brian_polling_manager/error_report/report.py | 13 +++++++++---- test/error_report/test_error_report.py | 10 +++++----- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/brian_polling_manager/error_report/cli.py b/brian_polling_manager/error_report/cli.py index f45becd..2fa731e 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 06d6f30..190d08e 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 c296848..3c1764a 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 721f19a..78c4f33 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 -- GitLab