From ff26ac4c5aae385c4643d84390b1263ec7b01fa6 Mon Sep 17 00:00:00 2001 From: Thibault Lestang Date: Wed, 26 Jul 2023 21:08:10 +0100 Subject: [PATCH 1/8] raise InvalidLocationError if unknown postcode This is assuming that the _only_ way of getting None as a response object is by supplying an unknown postcode to the carbonintensity api --- cats/CI_api_interface.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/cats/CI_api_interface.py b/cats/CI_api_interface.py index 4a97756..74589cb 100644 --- a/cats/CI_api_interface.py +++ b/cats/CI_api_interface.py @@ -4,8 +4,11 @@ from .forecast import CarbonIntensityPointEstimate +class InvalidLocationError(Exception): + pass + + APIInterface = namedtuple('APIInterface', ['get_request_url', 'parse_response_data']) -# TODO add a validation function to check the validity of the --location argument def ciuk_request_url(timestamp: datetime, postcode: str): # This transformation is specific to the CI-UK API. @@ -39,6 +42,20 @@ def ciuk_parse_response_data(response: dict): :param response: :return: """ + def invalid_code(r: dict): + try: + return "postcode" in r['error']['message'] + except KeyError: + return False + + # carbonintensity.org.uk API's response behavior is inconsistent + # with regards to bad postcodes. Passing a single character in the + # URL will give a 400 error code with a useful message. However + # giving a longer string, not matching a valid postcode, lead to + # no response at all. + if (not response) or invalid_code(response): + raise InvalidLocationError + datefmt = "%Y-%m-%dT%H:%MZ" return [ CarbonIntensityPointEstimate( From 560c2147227141b3d4124b27ffc05491baac71e2 Mon Sep 17 00:00:00 2001 From: Thibault Lestang Date: Wed, 26 Jul 2023 21:10:54 +0100 Subject: [PATCH 2/8] Leave location validation to the API --- cats/__init__.py | 4 ++-- cats/check_clean_arguments.py | 13 ------------- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/cats/__init__.py b/cats/__init__.py index ad1ad04..38b45ba 100644 --- a/cats/__init__.py +++ b/cats/__init__.py @@ -4,7 +4,7 @@ import yaml import sys -from .check_clean_arguments import validate_jobinfo, validate_duration, validate_location +from .check_clean_arguments import validate_jobinfo, validate_duration from .optimise_starttime import get_avg_estimates # noqa: F401 from .CI_api_interface import API_interfaces from .CI_api_query import get_CI_forecast # noqa: F401 @@ -90,7 +90,7 @@ def main(arguments=None): ## Location if args.location: - location = validate_location(args.location, choice_CI_API) + location = args.location sys.stderr.write(f"Using location provided: {location}\n") elif "location" in config.keys(): location = validate_location(config["location"], choice_CI_API) diff --git a/cats/check_clean_arguments.py b/cats/check_clean_arguments.py index 05da92b..d22d1aa 100644 --- a/cats/check_clean_arguments.py +++ b/cats/check_clean_arguments.py @@ -54,16 +54,3 @@ def validate_duration(duration): raise ValueError("--duration needs to be positive (number of minutes)") return duration_int - -def validate_location(location, choice_CI_API): - if choice_CI_API == 'carbonintensity.org.uk': - # in case the long format of the postcode is provided: - loc_cleaned = location.split()[0] - - # check that it's between 2 and 4 characters long - # TODO Check that it's a valid postcode for the API/country of interest - if (len(loc_cleaned) < 2) or (len(loc_cleaned) > 4): - raise ValueError(f"{location} is an invalid UK postcode. Only the first part of the postcode is expected (e.g. M15).") - else: - loc_cleaned = location - return loc_cleaned From 27d11579f9a90ed7de8e87519a18cd250e740bfa Mon Sep 17 00:00:00 2001 From: Thibault Lestang Date: Wed, 26 Jul 2023 21:11:55 +0100 Subject: [PATCH 3/8] exit program execution on invalid location --- cats/__init__.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cats/__init__.py b/cats/__init__.py index 38b45ba..7dd948c 100644 --- a/cats/__init__.py +++ b/cats/__init__.py @@ -6,7 +6,7 @@ from .check_clean_arguments import validate_jobinfo, validate_duration from .optimise_starttime import get_avg_estimates # noqa: F401 -from .CI_api_interface import API_interfaces +from .CI_api_interface import API_interfaces, InvalidLocationError from .CI_api_query import get_CI_forecast # noqa: F401 from .carbonFootprint import greenAlgorithmsCalculator @@ -109,7 +109,11 @@ def main(arguments=None): ######################## CI_API_interface = API_interfaces[choice_CI_API] - CI_forecast = get_CI_forecast(location, CI_API_interface) + try: + CI_forecast = get_CI_forecast(location, CI_API_interface) + except InvalidLocationError: + sys.stderr.write(f"Error: unknown location {location}\n") + sys.exit(1) ############################# ## Find optimal start time ## From ab7ce0d7da6c28ffae76d19b67814872fbbcd689 Mon Sep 17 00:00:00 2001 From: Thibault Lestang Date: Wed, 26 Jul 2023 21:20:28 +0100 Subject: [PATCH 4/8] test: InvalidLocationError is raised on bad postcode --- tests/test_api_query.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/test_api_query.py b/tests/test_api_query.py index ae34d94..8fb745d 100644 --- a/tests/test_api_query.py +++ b/tests/test_api_query.py @@ -1,5 +1,7 @@ +import pytest + import cats -from cats.CI_api_interface import API_interfaces +from cats.CI_api_interface import API_interfaces, InvalidLocationError from cats.forecast import CarbonIntensityPointEstimate def test_api_call(): @@ -13,3 +15,12 @@ def test_api_call(): assert isinstance(response, list) for item in response: assert isinstance(item, CarbonIntensityPointEstimate) + +def test_bad_postcode(): + api_interface = API_interfaces["carbonintensity.org.uk"] + + with pytest.raises(InvalidLocationError): + response = cats.CI_api_query.get_CI_forecast('OX48', api_interface) + + with pytest.raises(InvalidLocationError): + response = cats.CI_api_query.get_CI_forecast('A', api_interface) From 6c608c6b3f00b8a442786d71914f8ee75df76ca8 Mon Sep 17 00:00:00 2001 From: Thibault Lestang Date: Wed, 26 Jul 2023 21:22:37 +0100 Subject: [PATCH 5/8] Accept two part postcodes (space-separated) --- cats/CI_api_interface.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cats/CI_api_interface.py b/cats/CI_api_interface.py index 74589cb..145540e 100644 --- a/cats/CI_api_interface.py +++ b/cats/CI_api_interface.py @@ -27,7 +27,7 @@ def ciuk_request_url(timestamp: datetime, postcode: str): "https://api.carbonintensity.org.uk/regional/intensity/" + dt.strftime("%Y-%m-%dT%H:%MZ") + "/fw48h/postcode/" - + postcode + + postcode.split()[0] ) From 997548f4b9f239bc75799fb7a0a988ecb207cfb4 Mon Sep 17 00:00:00 2001 From: Thibault Lestang Date: Wed, 26 Jul 2023 21:51:36 +0100 Subject: [PATCH 6/8] Remove extra calls to 'validate_location' --- cats/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cats/__init__.py b/cats/__init__.py index 7dd948c..73e0f41 100644 --- a/cats/__init__.py +++ b/cats/__init__.py @@ -93,12 +93,12 @@ def main(arguments=None): location = args.location sys.stderr.write(f"Using location provided: {location}\n") elif "location" in config.keys(): - location = validate_location(config["location"], choice_CI_API) + location = config["location"] sys.stderr.write(f"Using location from config file: {location}\n") else: r = requests.get("https://ipapi.co/json").json() postcode = r["postal"] - location = validate_location(postcode, choice_CI_API) + location = postcode sys.stderr.write(f"WARNING: location not provided. Estimating location from IP address: {location}.\n") ## Duration From b68b23c3cc09e79b66fa3ca2274ce255dd909bae Mon Sep 17 00:00:00 2001 From: Thibault Lestang Date: Tue, 1 Aug 2023 15:33:23 +0100 Subject: [PATCH 7/8] Specify location format on InvalidLocationError (UK) --- cats/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cats/__init__.py b/cats/__init__.py index 73e0f41..b673794 100644 --- a/cats/__init__.py +++ b/cats/__init__.py @@ -113,6 +113,10 @@ def main(arguments=None): CI_forecast = get_CI_forecast(location, CI_API_interface) except InvalidLocationError: sys.stderr.write(f"Error: unknown location {location}\n") + sys.stderr.write( + "Location should be be specified as the outward code,\n" + "for example 'SW7' for postcode 'SW7 EAZ'.\n" + ) sys.exit(1) ############################# From 8041bccbfce99bfa068ff9bc4b6c035d25deb041 Mon Sep 17 00:00:00 2001 From: Thibault Lestang Date: Tue, 1 Aug 2023 15:41:02 +0100 Subject: [PATCH 8/8] Truncate long form postcode and warn user (UK) --- cats/CI_api_interface.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cats/CI_api_interface.py b/cats/CI_api_interface.py index 145540e..dc3e3fb 100644 --- a/cats/CI_api_interface.py +++ b/cats/CI_api_interface.py @@ -1,3 +1,4 @@ +import sys from collections import namedtuple from datetime import datetime @@ -23,11 +24,16 @@ def ciuk_request_url(timestamp: datetime, postcode: str): else: dt = timestamp.replace(minute=1, second=0, microsecond=0) + if (len(postcode) > 4): + sys.stderr.write(f"Warning: truncating postcode {postcode} to ") + postcode = postcode[:-3].strip() + sys.stderr.write(f"{postcode}.\n") + return ( "https://api.carbonintensity.org.uk/regional/intensity/" + dt.strftime("%Y-%m-%dT%H:%MZ") + "/fw48h/postcode/" - + postcode.split()[0] + + postcode )