From c0c7e546a304fcca026eafc8dc9b1abacc44a86a Mon Sep 17 00:00:00 2001 From: Chris Berkhout Date: Sun, 11 Jul 2021 15:50:59 +0200 Subject: [PATCH] Better logging of critical exceptions. Isolate tests and error handling for ECB. --- src/pricehist/fetch.py | 10 +- src/pricehist/logger.py | 17 +- src/pricehist/sources/ecb.py | 51 ++- tests/pricehist/sources/test_ecb.py | 225 ++++++++--- .../sources/test_ecb/eurofxref-hist-empty.xml | 9 + .../test_ecb/eurofxref-hist-partial.xml | 358 ++++++++++++++++++ 6 files changed, 604 insertions(+), 66 deletions(-) create mode 100644 tests/pricehist/sources/test_ecb/eurofxref-hist-empty.xml create mode 100644 tests/pricehist/sources/test_ecb/eurofxref-hist-partial.xml diff --git a/src/pricehist/fetch.py b/src/pricehist/fetch.py index f57f21b..e1cc341 100644 --- a/src/pricehist/fetch.py +++ b/src/pricehist/fetch.py @@ -1,6 +1,9 @@ import logging +import sys from datetime import date, datetime, timedelta +from pricehist import exceptions + def fetch(series, source, output, invert: bool, quantize: int, fmt) -> str: if series.start < source.start(): @@ -9,7 +12,12 @@ def fetch(series, source, output, invert: bool, quantize: int, fmt) -> str: f"source start date of {source.start()}." ) - series = source.fetch(series) + try: + series = source.fetch(series) + except exceptions.SourceError as e: + logging.debug("Critical exception encountered", exc_info=e) + logging.critical(str(e)) + sys.exit(1) if len(series.prices) == 0: logging.warn(f"No data found for the interval [{series.start}--{series.end}].") diff --git a/src/pricehist/logger.py b/src/pricehist/logger.py index 648598f..3a6143f 100644 --- a/src/pricehist/logger.py +++ b/src/pricehist/logger.py @@ -4,11 +4,18 @@ import sys class Formatter(logging.Formatter): def format(self, record): - message = record.msg % record.args if record.args else record.msg - if record.levelno == logging.INFO: - return message - else: - return f"{record.levelname} {message}" + s = record.msg % record.args if record.args else record.msg + + if record.exc_info: + record.exc_text = self.formatException(record.exc_info) + if s[-1:] != "\n": + s = s + "\n" + s = s + "\n".join([f" {line}" for line in record.exc_text.splitlines()]) + + if record.levelno != logging.INFO: + s = "\n".join([f"{record.levelname} {line}" for line in s.splitlines()]) + + return s def init(): diff --git a/src/pricehist/sources/ecb.py b/src/pricehist/sources/ecb.py index c5c1f96..16cb27d 100644 --- a/src/pricehist/sources/ecb.py +++ b/src/pricehist/sources/ecb.py @@ -1,13 +1,11 @@ import dataclasses -import logging -import sys from datetime import datetime, timedelta from decimal import Decimal import requests from lxml import etree -from pricehist import isocurrencies +from pricehist import exceptions, isocurrencies from pricehist.price import Price from .basesource import BaseSource @@ -36,19 +34,17 @@ class ECB(BaseSource): return "" def symbols(self): - root = self._data(more_than_90_days=True) - nodes = root.cssselect("[currency]") - currencies = sorted(set([n.attrib["currency"] for n in nodes])) + quotes = self._quotes() iso = isocurrencies.by_code() - return [(f"EUR/{c}", f"Euro against {iso[c].name}") for c in currencies] + return [ + (f"EUR/{c}", f"Euro against {iso[c].name if c in iso else c}") + for c in quotes + ] def fetch(self, series): - if series.base != "EUR": # EUR is the only valid base. - logging.critical( - f"Invalid pair '{'/'.join([series.base, series.quote])}'. " - f"Run 'pricehist source {self.id()} --symbols' to list valid pairs." - ) - sys.exit(1) + if series.base != "EUR" or not series.quote: # EUR is the only valid base. + raise exceptions.InvalidPair(series.base, series.quote, self) + almost_90_days_ago = (datetime.now().date() - timedelta(days=85)).isoformat() root = self._data(series.start < almost_90_days_ago) @@ -58,12 +54,24 @@ class ECB(BaseSource): for row in day.cssselect(f"[currency='{series.quote}']"): rate = Decimal(row.attrib["rate"]) all_rows.insert(0, (date, rate)) + + if not all_rows and series.quote not in self._quotes(): + raise exceptions.InvalidPair(series.base, series.quote, self) + selected = [ Price(d, r) for d, r in all_rows if d >= series.start and d <= series.end ] return dataclasses.replace(series, prices=selected) + def _quotes(self): + root = self._data(more_than_90_days=True) + nodes = root.cssselect("[currency]") + quotes = sorted(set([n.attrib["currency"] for n in nodes])) + if not quotes: + raise exceptions.ResponseParsingError("Expected data not found") + return quotes + def _data(self, more_than_90_days=False): url_base = "https://www.ecb.europa.eu/stats/eurofxref" if more_than_90_days: @@ -71,6 +79,19 @@ class ECB(BaseSource): else: source_url = f"{url_base}/eurofxref-hist-90d.xml" # last 90 days - response = self.log_curl(requests.get(source_url)) - root = etree.fromstring(response.content) + try: + response = self.log_curl(requests.get(source_url)) + except Exception as e: + raise exceptions.RequestError(str(e)) from e + + try: + response.raise_for_status() + except Exception as e: + raise exceptions.BadResponse(str(e)) from e + + try: + root = etree.fromstring(response.content) + except Exception as e: + raise exceptions.ResponseParsingError(str(e)) from e + return root diff --git a/tests/pricehist/sources/test_ecb.py b/tests/pricehist/sources/test_ecb.py index 4b9f0bf..912f933 100644 --- a/tests/pricehist/sources/test_ecb.py +++ b/tests/pricehist/sources/test_ecb.py @@ -1,14 +1,18 @@ +import logging +import os +from datetime import datetime, timedelta from decimal import Decimal +from pathlib import Path import pytest +import requests +import responses +from pricehist import exceptions, isocurrencies from pricehist.price import Price from pricehist.series import Series from pricehist.sources.ecb import ECB -# import responses -# @responses.activate - def in_log(caplog, levelname, substr): return any( @@ -26,73 +30,204 @@ def type(src): return src.types()[0] +@pytest.fixture +def xml(): + dir = Path(os.path.splitext(__file__)[0]) + return (dir / "eurofxref-hist-partial.xml").read_text() + + +@pytest.fixture +def empty_xml(): + dir = Path(os.path.splitext(__file__)[0]) + return (dir / "eurofxref-hist-empty.xml").read_text() + + +@pytest.fixture +def requests_mock(): + with responses.RequestsMock() as mock: + yield mock + + +@pytest.fixture +def response_ok(requests_mock, xml): + requests_mock.add( + responses.GET, + "https://www.ecb.europa.eu/stats/eurofxref/eurofxref-hist.xml", + body=xml, + status=200, + ) + yield requests_mock + + +@pytest.fixture +def response_ok_90d(requests_mock, xml): + requests_mock.add( + responses.GET, + "https://www.ecb.europa.eu/stats/eurofxref/eurofxref-hist-90d.xml", + body=xml, + status=200, + ) + yield requests_mock + + +@pytest.fixture +def response_empty_xml(requests_mock, empty_xml): + requests_mock.add( + responses.GET, + "https://www.ecb.europa.eu/stats/eurofxref/eurofxref-hist.xml", + body=empty_xml, + status=200, + ) + yield requests_mock + + def test_normalizesymbol(src): assert src.normalizesymbol("eur") == "EUR" assert src.normalizesymbol("symbol") == "SYMBOL" -@pytest.mark.live -def test_known_pair(src, type): - series = src.fetch(Series("EUR", "AUD", type, "2021-01-11", "2021-01-22")) - assert series.prices[0] == Price("2021-01-11", Decimal("1.5783")) - assert series.prices[-1] == Price("2021-01-22", Decimal("1.577")) - assert len(series.prices) == 10 +def test_metadata(src): + assert isinstance(src.id(), str) + assert len(src.id()) > 0 + + assert isinstance(src.name(), str) + assert len(src.name()) > 0 + + assert isinstance(src.description(), str) + assert len(src.description()) > 0 + + assert isinstance(src.source_url(), str) + assert src.source_url().startswith("http") + + assert datetime.strptime(src.start(), "%Y-%m-%d") + + assert isinstance(src.types(), list) + assert len(src.types()) > 0 + assert isinstance(src.types()[0], str) + assert len(src.types()[0]) > 0 + + assert isinstance(src.notes(), str) -@pytest.mark.live -def test_long_hist_from_start(src, type): - series = src.fetch(Series("EUR", "AUD", type, src.start(), "2021-07-01")) +def test_symbols(src, response_ok): + syms = src.symbols() + assert ("EUR/AUD", "Euro against Australian Dollar") in syms + assert len(syms) > 40 + + +def test_requests_logged_for_symbols(src, response_ok, caplog): + with caplog.at_level(logging.DEBUG): + src.symbols() + assert in_log(caplog, "DEBUG", " curl ") + + +def test_symbols_not_in_iso_data(src, response_ok, monkeypatch): + iso = isocurrencies.by_code() + del iso["AUD"] + monkeypatch.setattr(isocurrencies, "by_code", lambda: iso) + syms = src.symbols() + assert ("EUR/AUD", "Euro against AUD") in syms + + +def test_symbols_not_found(src, response_empty_xml): + with pytest.raises(exceptions.ResponseParsingError) as e: + src.symbols() + assert "data not found" in str(e.value) + + +def test_known_pair(src, type, response_ok): + series = src.fetch(Series("EUR", "AUD", type, "2021-01-04", "2021-01-08")) + assert series.prices[0] == Price("2021-01-04", Decimal("1.5928")) + assert series.prices[-1] == Price("2021-01-08", Decimal("1.5758")) + assert len(series.prices) == 5 + + +def test_requests_logged_for_fetch(src, response_ok, caplog): + with caplog.at_level(logging.DEBUG): + src.fetch(Series("EUR", "AUD", type, "2021-01-04", "2021-01-08")) + assert in_log(caplog, "DEBUG", " curl ") + + +def test_recent_interval_uses_90d_data(src, type, response_ok_90d): + today = datetime.now().date() + start = (today - timedelta(days=80)).isoformat() + end = today.isoformat() + src.fetch(Series("EUR", "AUD", type, start, end)) + assert len(response_ok_90d.calls) > 0 + + +def test_long_hist_from_start(src, type, response_ok): + series = src.fetch(Series("EUR", "AUD", type, src.start(), "2021-01-08")) assert series.prices[0] == Price("1999-01-04", Decimal("1.91")) - assert series.prices[-1] == Price("2021-07-01", Decimal("1.5836")) - assert len(series.prices) == 5759 + assert series.prices[-1] == Price("2021-01-08", Decimal("1.5758")) + assert len(series.prices) > 9 -@pytest.mark.live -def test_from_before_start(src, type): +def test_from_before_start(src, type, response_ok): series = src.fetch(Series("EUR", "AUD", type, "1998-12-01", "1999-01-10")) assert series.prices[0] == Price("1999-01-04", Decimal("1.91")) assert series.prices[-1] == Price("1999-01-08", Decimal("1.8406")) assert len(series.prices) == 5 -@pytest.mark.live -def test_to_future(src, type): - series = src.fetch(Series("EUR", "AUD", type, "2021-07-01", "2100-01-01")) +def test_to_future(src, type, response_ok): + series = src.fetch(Series("EUR", "AUD", type, "2021-01-04", "2100-01-01")) assert len(series.prices) > 0 -@pytest.mark.live -def test_known_pair_no_data(src, type): - series = src.fetch(Series("EUR", "ROL", type, "2020-01-01", "2021-01-01")) +def test_known_pair_no_data(src, type, response_ok): + series = src.fetch(Series("EUR", "ROL", type, "2021-01-04", "2021-02-08")) assert len(series.prices) == 0 -def test_non_eur_base(src, type, caplog): - with pytest.raises(SystemExit) as e: - src.fetch(Series("USD", "AUD", type, "2021-01-01", "2021-02-01")) - assert e.value.code == 1 - assert in_log(caplog, "CRITICAL", "Invalid pair") +def test_non_eur_base(src, type): + with pytest.raises(exceptions.InvalidPair): + src.fetch(Series("USD", "AUD", type, "2021-01-04", "2021-01-08")) -@pytest.mark.xfail -@pytest.mark.live -def test_unknown_quote(src, type, caplog): - with pytest.raises(SystemExit) as e: - src.fetch(Series("EUR", "XZY", type, "2021-01-01", "2021-02-01")) - assert e.value.code == 1 - assert in_log(caplog, "CRITICAL", "Invalid pair") +def test_unknown_quote(src, type, response_ok): + with pytest.raises(exceptions.InvalidPair): + src.fetch(Series("EUR", "XZY", type, "2021-01-04", "2021-01-08")) -@pytest.mark.xfail -def test_no_quote(src, type, caplog): - with pytest.raises(SystemExit) as e: - src.fetch(Series("EUR", "", type, "2021-01-01", "2021-02-01")) - assert e.value.code == 1 - assert in_log(caplog, "CRITICAL", "Invalid pair") +def test_no_quote(src, type): + with pytest.raises(exceptions.InvalidPair): + src.fetch(Series("EUR", "", type, "2021-01-04", "2021-01-08")) -def test_unknown_pair(src, type, caplog): - with pytest.raises(SystemExit) as e: - src.fetch(Series("ABC", "XZY", type, "2021-01-01", "2021-02-01")) - assert e.value.code == 1 - assert in_log(caplog, "CRITICAL", "Invalid pair") +def test_unknown_pair(src, type): + with pytest.raises(exceptions.InvalidPair): + src.fetch(Series("ABC", "XZY", type, "2021-01-04", "2021-01-08")) + + +def test_network_issue(src, type, requests_mock): + requests_mock.add( + responses.GET, + "https://www.ecb.europa.eu/stats/eurofxref/eurofxref-hist.xml", + body=requests.exceptions.ConnectionError("Network issue"), + ) + with pytest.raises(exceptions.RequestError) as e: + src.fetch(Series("EUR", "AUD", type, "2021-01-04", "2021-01-08")) + assert "Network issue" in str(e.value) + + +def test_bad_status(src, type, requests_mock): + requests_mock.add( + responses.GET, + "https://www.ecb.europa.eu/stats/eurofxref/eurofxref-hist.xml", + status=500, + ) + with pytest.raises(exceptions.BadResponse) as e: + src.fetch(Series("EUR", "AUD", type, "2021-01-04", "2021-01-08")) + assert "Server Error" in str(e.value) + + +def test_parsing_error(src, type, requests_mock): + requests_mock.add( + responses.GET, + "https://www.ecb.europa.eu/stats/eurofxref/eurofxref-hist.xml", + body="NOT XML", + ) + with pytest.raises(exceptions.ResponseParsingError) as e: + src.fetch(Series("EUR", "AUD", type, "2021-01-04", "2021-01-08")) + assert "while parsing data" in str(e.value) diff --git a/tests/pricehist/sources/test_ecb/eurofxref-hist-empty.xml b/tests/pricehist/sources/test_ecb/eurofxref-hist-empty.xml new file mode 100644 index 0000000..7483bd6 --- /dev/null +++ b/tests/pricehist/sources/test_ecb/eurofxref-hist-empty.xml @@ -0,0 +1,9 @@ + + + Reference rates + + European Central Bank + + + + diff --git a/tests/pricehist/sources/test_ecb/eurofxref-hist-partial.xml b/tests/pricehist/sources/test_ecb/eurofxref-hist-partial.xml new file mode 100644 index 0000000..3d6af0c --- /dev/null +++ b/tests/pricehist/sources/test_ecb/eurofxref-hist-partial.xml @@ -0,0 +1,358 @@ + + + Reference rates + + European Central Bank + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +