Merge pull request #77 from GreenScheduler/tl-configure

Move processing of cli args and config file to separate module

cats/__init__.py

@@ -7,13 +7,11 @@
 from datetime import timedelta
 from typing import Optional
 
-import requests
-import yaml
-
 from .carbonFootprint import Estimates, greenAlgorithmsCalculator
-from .check_clean_arguments import validate_duration, validate_jobinfo
-from .CI_api_interface import API_interfaces, InvalidLocationError
+from .check_clean_arguments import validate_jobinfo
+from .CI_api_interface import InvalidLocationError
 from .CI_api_query import get_CI_forecast  # noqa: F401
+from .configure import get_runtime_config
 from .forecast import CarbonIntensityAverageEstimate
 from .optimise_starttime import get_avg_estimates  # noqa: F401
 
@@ -145,7 +143,6 @@ def parse_arguments():
 class CATSOutput:
     """Carbon Aware Task Scheduler output"""
 
-    carbonIntensityAPI: str
     carbonIntensityNow: CarbonIntensityAverageEstimate
     carbonIntensityOptimal: CarbonIntensityAverageEstimate
     location: str
@@ -198,65 +195,12 @@ def main(arguments=None):
             "      specify the scheduler with the -s or --scheduler option"
         )
         sys.exit(1)
-
-    ##################################
-    ## Validate and clean arguments ##
-    ##################################
-
-    ## config file
-    if args.config:
-        # if path to config file provided, it is used
-        with open(args.config, "r") as f:
-            config = yaml.safe_load(f)
-        logging.info(f"Using provided config file: {args.config}\n")
-    else:
-        # if no path provided, look for `config.yml` in current directory
-        try:
-            with open("config.yml", "r") as f:
-                config = yaml.safe_load(f)
-            logging.info("Using config.yml found in current directory\n")
-        except FileNotFoundError:
-            config = {}
-            logging.warning("config file not found")
-
-    ## CI API choice
-    list_CI_APIs = ["carbonintensity.org.uk"]
-
-    choice_CI_API = "carbonintensity.org.uk"  # default value
-    if "api" in config.keys():
-        choice_CI_API = config["api"]
-    if args.api:
-        choice_CI_API = args.api
-
-    if choice_CI_API not in list_CI_APIs:
-        raise ValueError(
-            f"{choice_CI_API} is not a valid API choice, it needs to be one of {list_CI_APIs}."
-        )
-    logging.info(f"Using {choice_CI_API} for carbon intensity forecasts\n")
-
-    ## Location
-    if args.location:
-        location = args.location
-        logging.info(f"Using location provided: {location}")
-    elif "location" in config.keys():
-        location = config["location"]
-        logging.info(f"Using location from config file: {location}")
-    else:
-        r = requests.get("https://ipapi.co/json").json()
-        postcode = r["postal"]
-        location = postcode
-        logging.warning(
-            f"location not provided. Estimating location from IP address: {location}."
-        )
-
-    ## Duration
-    duration = validate_duration(args.duration)
+    config, CI_API_interface, location, duration = get_runtime_config(args)
 
     ########################
     ## Obtain CI forecast ##
     ########################
 
-    CI_API_interface = API_interfaces[choice_CI_API]
     try:
         CI_forecast = get_CI_forecast(location, CI_API_interface)
     except InvalidLocationError:
@@ -274,7 +218,7 @@ def main(arguments=None):
     # Find best possible average carbon intensity, along
     # with corresponding job start time.
     now_avg, best_avg = get_avg_estimates(CI_forecast, duration=duration)
-    output = CATSOutput(choice_CI_API, now_avg, best_avg, location, "GBR")
+    output = CATSOutput(now_avg, best_avg, location, "GBR")
 
     ################################
     ## Calculate carbon footprint ##

cats/check_clean_arguments.py

@@ -45,18 +45,3 @@ def validate_jobinfo(jobinfo: str, expected_partition_names):
             return {}
 
     return info
-
-
-def validate_duration(duration):
-    # make sure it can be converted to integer
-    try:
-        duration_int = int(duration)
-    except ValueError:
-        raise ValueError(
-            "--duration needs to be an integer or float (number of minutes)"
-        )
-    # make sure it's positive
-    if duration_int <= 0:
-        raise ValueError("--duration needs to be positive (number of minutes)")
-
-    return duration_int

cats/configure.py

@@ -0,0 +1,114 @@
+"""This module exports a function :py:func:`configure
+<cats.configure.configure>` that processes both command line arguments
+and configuration file. This function returns a runtime configuration
+for cats to make a request to a carcon intensity forecast provider.  A
+runtime configuration consits of:
+
+- location (postcode)
+- job duration
+- Interface to carbon intensity forecast provider (See TODO)
+
+"""
+
+import logging
+import sys
+from collections.abc import Mapping
+from typing import Any
+
+import requests
+import yaml
+
+from .CI_api_interface import API_interfaces, APIInterface
+
+__all__ = ["get_runtime_config"]
+
+
+def get_runtime_config(args) -> tuple[dict, APIInterface, str, int]:
+    """Return the runtime cats configuration from list of command line
+    arguments and content of configuration file.
+
+    Returns a tupe containing a dictionary reprensenting the
+    configuration file, an instance of :py:class:`APIInterface
+    <cats.CI_api_interface.APIInterface>`, the location as a string
+    and the duration in minutes as an integer.
+
+    :param args: Command line arguments
+    :return: Runtime cats configuration
+    :rtype: tuple[dict, APIInterface, str, int]
+    :raises ValueError: If job duration cannot be interpreted as a positive integer.
+
+    """
+    configmapping = config_from_file(configpath=args.config)
+    CI_API_interface = CI_API_from_config_or_args(args, configmapping)
+    location = get_location_from_config_or_args(args, configmapping)
+
+    msg = "Job duration must be a positive integer (number of minutes)"
+    try:
+        duration = int(args.duration)
+    except ValueError:
+        logging.eror(msg)
+        raise ValueError
+    if duration <= 0:
+        logging.error(msg)
+        raise ValueError
+
+    return configmapping, CI_API_interface, location, duration
+
+
+def config_from_file(configpath="") -> Mapping[str, Any]:
+    if configpath:
+        # if path to config file provided, it is used
+        with open(configpath, "r") as f:
+            return yaml.safe_load(f)
+        logging.info(f"Using provided config file: {configpath}\n")
+    else:
+        # if no path provided, look for `config.yml` in current directory
+        try:
+            with open("config.yml", "r") as f:
+                return yaml.safe_load(f)
+            logging.info("Using config.yml found in current directory\n")
+        except FileNotFoundError:
+            logging.warning("config file not found")
+            return {}
+
+
+def CI_API_from_config_or_args(args, config) -> APIInterface:
+    try:
+        api = args.api if args.api else config["api"]
+    except KeyError:
+        api = "carbonintensity.org.uk"  # default value
+        logging.warning(f"Unspecified carbon intensity forecast service, using {api}")
+    try:
+        return API_interfaces[api]
+    except KeyError:
+        logging.error(
+            f"Error: {api} is not a valid API choice. It must be one of " "\n".join(
+                API_interfaces.keys()
+            )
+        )
+
+
+def get_location_from_config_or_args(args, config) -> str:
+    if args.location:
+        location = args.location
+        logging.info(f"Using location provided from command line: {location}")
+        return location
+    if "location" in config.keys():
+        location = config["location"]
+        logging.info(f"Using location from config file: {location}")
+        return location
+
+    r = requests.get("https://ipapi.co/json/")
+    if r.status_code != 200:
+        logging.error(
+            "Could not get location from ipapi.co.\n"
+            f"Got Error {r.status_code} - {r.json()['reason']}\n"
+            f"{r.json()['message']}"
+        )
+        sys.exit(1)
+    location = r.json()["postal"]
+    assert location
+    logging.warning(
+        f"location not provided. Estimating location from IP address: {location}."
+    )
+    return location

docs/source/api-reference.rst

@@ -23,6 +23,12 @@ Modules
 .. automodule:: cats.__main__
     :members:
 
+``cats.configure``
+^^^^^^^^^^^^^^^^^^
+
+.. automodule:: cats.configure
+   :members:
+
 ``cats.CI_api_interface``
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 

tests/test_configure.py

@@ -0,0 +1,95 @@
+import os
+from contextlib import contextmanager
+from unittest.mock import Mock, patch
+
+import pytest
+import yaml
+
+from cats import parse_arguments
+from cats.CI_api_interface import API_interfaces
+from cats.configure import (
+    CI_API_from_config_or_args,
+    config_from_file,
+    get_location_from_config_or_args,
+)
+
+CATS_CONFIG = {
+    "location": "EH8",
+    "api": "carbonintensity.org.uk",
+}
+
+
+@contextmanager
+def change_dir(p):
+    current_dir = os.getcwd()
+    os.chdir(p)
+    yield
+    os.chdir(current_dir)
+
+
+@pytest.fixture
+def local_config_file(tmp_path_factory):
+    p = tmp_path_factory.mktemp("temp") / "config.yml"
+    with open(p, "w") as stream:
+        yaml.dump(CATS_CONFIG, stream)
+    return p.parent
+
+
+def test_config_from_file():
+    missing_file = "missing.yaml"
+    with pytest.raises(FileNotFoundError):
+        config_from_file(missing_file)
+        config_from_file()
+
+
+def test_config_from_file_default(local_config_file):
+    with change_dir(local_config_file):
+        configmapping = config_from_file()
+    assert configmapping == CATS_CONFIG
+
+
+@patch("cats.configure.requests")
+def test_get_location_from_config_or_args(mock_requests):
+    expected_location = "SW7"
+    mock_requests.get.return_value = Mock(
+        **{
+            "status_code": 200,
+            "json.return_value": {"postal": expected_location},
+        }
+    )
+
+    args = parse_arguments().parse_args(
+        ["--location", expected_location, "--duration", "1"]
+    )
+    location = get_location_from_config_or_args(args, CATS_CONFIG)
+    assert location == expected_location
+
+    args = parse_arguments().parse_args(["--duration", "1"])
+    location = get_location_from_config_or_args(args, CATS_CONFIG)
+    assert location == CATS_CONFIG["location"]
+
+    args = parse_arguments().parse_args(["--duration", "1"])
+    config = {}
+    location = get_location_from_config_or_args(args, config)
+    mock_requests.get.assert_called_once()
+    assert location == expected_location
+
+
+def get_CI_API_from_config_or_args(args, config):
+    expected_interface = API_interfaces["carbonintensity.org.uk"]
+    args = parse_arguments().parse_args(
+        ["--api", "carbonintensity.org.uk", "--duration", "1"]
+    )
+    API_interface = CI_API_from_config_or_args(args, CATS_CONFIG)
+    assert API_interface == expected_interface
+
+    args = parse_arguments().parse_args(["--duration", "1"])
+    API_interface = CI_API_from_config_or_args(args, CATS_CONFIG)
+    assert API_interface == expected_interface
+
+    args = parse_arguments().parse_args(
+        ["--api", "doesnotexist.co.uk", "--duration", "1"]
+    )
+    API_interface = CI_API_from_config_or_args(args, CATS_CONFIG)
+    with pytest.raises(KeyError):
+        CI_API_from_config_or_args(args, CATS_CONFIG)