refactor: migrate api_client.py prints to structured logging
Replace all print() calls with logger.info/logger.error using lazy % formatting. Add test verifying error path emits ERROR log. U2 of P2-001 (replace print with logging)
This commit is contained in:
+17
-12
@@ -1,4 +1,5 @@
|
|||||||
# api_client.py (complete updated version)
|
# api_client.py (complete updated version)
|
||||||
|
import logging
|
||||||
import requests
|
import requests
|
||||||
import json
|
import json
|
||||||
import re
|
import re
|
||||||
@@ -8,6 +9,8 @@ from config import config
|
|||||||
import time
|
import time
|
||||||
from collections import defaultdict
|
from collections import defaultdict
|
||||||
|
|
||||||
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
|
|
||||||
class TweedeKamerAPI:
|
class TweedeKamerAPI:
|
||||||
def __init__(self):
|
def __init__(self):
|
||||||
@@ -42,18 +45,18 @@ class TweedeKamerAPI:
|
|||||||
voting_records, besluit_meta = self._get_voting_records(
|
voting_records, besluit_meta = self._get_voting_records(
|
||||||
start_date, end_date, limit
|
start_date, end_date, limit
|
||||||
)
|
)
|
||||||
print(f"Fetched {len(voting_records)} voting records from API")
|
logger.info("Fetched %d voting records from API", len(voting_records))
|
||||||
|
|
||||||
# Group by Besluit_Id (decision/motion) and get motion details
|
# Group by Besluit_Id (decision/motion) and get motion details
|
||||||
motions = self._process_voting_records(
|
motions = self._process_voting_records(
|
||||||
voting_records, besluit_meta, skip_details=skip_details
|
voting_records, besluit_meta, skip_details=skip_details
|
||||||
)
|
)
|
||||||
print(f"Processed into {len(motions)} unique motions")
|
logger.info("Processed into %d unique motions", len(motions))
|
||||||
|
|
||||||
return motions
|
return motions
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
print(f"Error fetching motions from API: {e}")
|
logger.error("Error fetching motions from API: %s", e)
|
||||||
return []
|
return []
|
||||||
|
|
||||||
def _get_voting_records(
|
def _get_voting_records(
|
||||||
@@ -132,16 +135,18 @@ class TweedeKamerAPI:
|
|||||||
break # last page
|
break # last page
|
||||||
skip += page_size
|
skip += page_size
|
||||||
|
|
||||||
print(
|
logger.info(
|
||||||
f"Retrieved {len(all_records)} voting records from {len(besluit_meta)} decisions"
|
"Retrieved %d voting records from %d decisions",
|
||||||
|
len(all_records),
|
||||||
|
len(besluit_meta),
|
||||||
)
|
)
|
||||||
return all_records, besluit_meta
|
return all_records, besluit_meta
|
||||||
|
|
||||||
except requests.exceptions.RequestException as e:
|
except requests.exceptions.RequestException as e:
|
||||||
print(f"API request failed: {e}")
|
logger.error("API request failed: %s", e)
|
||||||
if hasattr(e, "response") and e.response is not None:
|
if hasattr(e, "response") and e.response is not None:
|
||||||
print(f"Response status: {e.response.status_code}")
|
logger.error("Response status: %d", e.response.status_code)
|
||||||
print(f"Response text: {e.response.text[:500]}")
|
logger.error("Response text: %s", e.response.text[:500])
|
||||||
return all_records, besluit_meta # return whatever we got before failure
|
return all_records, besluit_meta # return whatever we got before failure
|
||||||
|
|
||||||
def _process_voting_records(
|
def _process_voting_records(
|
||||||
@@ -336,7 +341,7 @@ class TweedeKamerAPI:
|
|||||||
}
|
}
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
print(f"Error getting motion details for {besluit_id}: {e}")
|
logger.error("Error getting motion details for %s: %s", besluit_id, e)
|
||||||
|
|
||||||
return None
|
return None
|
||||||
|
|
||||||
@@ -359,7 +364,7 @@ class TweedeKamerAPI:
|
|||||||
if ext_id:
|
if ext_id:
|
||||||
return ext_id
|
return ext_id
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
print(f"Error fetching ExterneIdentifier for zaak {zaak_id}: {e}")
|
logger.error("Error fetching ExterneIdentifier for zaak %s: %s", zaak_id, e)
|
||||||
|
|
||||||
return None
|
return None
|
||||||
|
|
||||||
@@ -412,7 +417,7 @@ class TweedeKamerAPI:
|
|||||||
return body if len(body) > 50 else None
|
return body if len(body) > 50 else None
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
print(f"Error fetching body text for {externe_identifier}: {e}")
|
logger.error("Error fetching body text for %s: %s", externe_identifier, e)
|
||||||
|
|
||||||
return None
|
return None
|
||||||
|
|
||||||
@@ -494,5 +499,5 @@ class TweedeKamerAPI:
|
|||||||
return len(data.get("value", [])) > 0
|
return len(data.get("value", [])) > 0
|
||||||
|
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
print(f"API connection test failed: {e}")
|
logger.error("API connection test failed: %s", e)
|
||||||
return False
|
return False
|
||||||
|
|||||||
@@ -0,0 +1,19 @@
|
|||||||
|
import logging
|
||||||
|
from unittest.mock import patch
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from api_client import TweedeKamerAPI
|
||||||
|
|
||||||
|
|
||||||
|
class TestTweedeKamerAPILogging:
|
||||||
|
def test_get_motions_error_logged(self, caplog):
|
||||||
|
api = TweedeKamerAPI()
|
||||||
|
caplog.set_level(logging.ERROR, logger="api_client")
|
||||||
|
|
||||||
|
with patch.object(api, "_get_voting_records", side_effect=Exception("network down")):
|
||||||
|
result = api.get_motions()
|
||||||
|
|
||||||
|
assert result == []
|
||||||
|
assert any("network down" in rec.message for rec in caplog.records)
|
||||||
|
assert any(rec.levelno == logging.ERROR for rec in caplog.records)
|
||||||
Reference in New Issue
Block a user