diff --git a/cli/.pylintrc b/cli/.pylintrc index 691ed1ec..48366260 100644 --- a/cli/.pylintrc +++ b/cli/.pylintrc @@ -4,4 +4,5 @@ good-names = k, v [MESSAGES CONTROL] # We currently have some older systems running this which # don't support f-strings yet -disable = C0209 +# Disable too-many-lines check because pylint on github seems to be miscounting lines in __init__.py +disable = C0209,too-many-lines diff --git a/cli/testflinger_cli/__init__.py b/cli/testflinger_cli/__init__.py index 821b02fb..603c722e 100644 --- a/cli/testflinger_cli/__init__.py +++ b/cli/testflinger_cli/__init__.py @@ -156,6 +156,11 @@ def __init__(self): or self.config.get("secret_key") or os.environ.get("TESTFLINGER_SECRET_KEY") ) + error_threshold = self.config.get( + "error_threshold" + or os.environ.get("TESTFLINGER_ERROR_THRESHOLD") + or 3 + ) # Allow config subcommand without worrying about server or client if ( @@ -168,7 +173,7 @@ def __init__(self): 'Server must start with "http://" or "https://" ' '- currently set to: "{}"'.format(server) ) - self.client = client.Client(server) + self.client = client.Client(server, error_threshold=error_threshold) def run(self): """Run the subcommand specified in command line arguments""" @@ -361,7 +366,12 @@ def status(self): job_state = self.get_job_state(self.args.job_id) if job_state != "unknown": self.history.update(self.args.job_id, job_state) - print(job_state) + print(job_state) + else: + print( + "Unable to retrieve job state from the server, check your " + "connection or try again later." + ) def cancel(self, job_id=None): """Tell the server to cancel a specified JOB_ID""" @@ -980,8 +990,8 @@ def get_job_state(self, job_id): "Received 404 error from server. Are you " "sure this is a testflinger server?" ) - except (IOError, ValueError): + except (IOError, ValueError) as exc: # For other types of network errors, or JSONDecodeError if we got # a bad return from get_status() - logger.warning("Unable to retrieve job state.") + logger.debug("Unable to retrieve job state: %s", exc) return "unknown" diff --git a/cli/testflinger_cli/client.py b/cli/testflinger_cli/client.py index a82bb1a0..635b7824 100644 --- a/cli/testflinger_cli/client.py +++ b/cli/testflinger_cli/client.py @@ -43,8 +43,10 @@ def __init__(self, status, msg=""): class Client: """Testflinger connection client""" - def __init__(self, server): + def __init__(self, server, error_threshold=3): self.server = server + self.error_count = 0 + self.error_threshold = error_threshold def get(self, uri_frag, timeout=15, headers=None): """Submit a GET request to the server @@ -56,14 +58,16 @@ def get(self, uri_frag, timeout=15, headers=None): uri = urllib.parse.urljoin(self.server, uri_frag) try: req = requests.get(uri, timeout=timeout, headers=headers) - except requests.exceptions.ConnectionError: - logger.error("Unable to communicate with specified server.") - raise - except IOError: - # This should catch all other timeout cases - logger.error( - "Timeout while trying to communicate with the server." - ) + self.error_count = 0 + except (IOError, requests.exceptions.ConnectionError) as exc: + self.error_count += 1 + if self.error_count % self.error_threshold == 0: + logger.warning( + "Error communicating with the server for the past %s " + "requests, but will continue to retry. Last error: %s", + self.error_count, + exc, + ) raise if req.status_code != 200: raise HTTPError(req.status_code) diff --git a/cli/testflinger_cli/tests/test_client.py b/cli/testflinger_cli/tests/test_client.py new file mode 100644 index 00000000..466c9824 --- /dev/null +++ b/cli/testflinger_cli/tests/test_client.py @@ -0,0 +1,47 @@ +# Copyright (C) 2024 Canonical +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +""" +Unit tests for the Client class +""" + +import logging +import pytest +import requests + +from testflinger_cli.client import Client + + +def test_get_error_threshold(caplog, requests_mock): + """Test that a warning is logged when error_threshold is reached""" + caplog.set_level(logging.WARNING) + requests_mock.get( + "http://testflinger/test", exc=requests.exceptions.ConnectionError + ) + client = Client("http://testflinger", error_threshold=3) + for _ in range(2): + with pytest.raises(requests.exceptions.ConnectionError): + client.get("test") + assert ( + "Error communicating with the server for the past" + not in caplog.text + ) + with pytest.raises(requests.exceptions.ConnectionError): + client.get("test") + assert ( + "Error communicating with the server for the past 3 requests" + in caplog.text + ) diff --git a/docs/reference/cli-config.rst b/docs/reference/cli-config.rst new file mode 100644 index 00000000..bdb9bd71 --- /dev/null +++ b/docs/reference/cli-config.rst @@ -0,0 +1,47 @@ +Command Line Interface Configuration +==================================== + +The Testflinger CLI can be configured on the command line using the parameters described in ``testflinger --help``. However, sometimes it also supports using environment variables, or reading values from a configuration file. This can be helpful for CI/CD environments, or for setting up a config with different default values read from a config file. + +The configuration file is read from ``$XDG_CONFIG_HOME/testflinger-cli.conf`` by default, but this can be overridden with the ``--configfile`` parameter. + +When a configuration variable is defined in multiple locations, Testflinger resolves the value by applying the following priority order, from highest to lowest: + +1. command-line parameter (for example, ``--server ``) + +2. configuration file + +3. environment variable + +You can show the current values stored in the config file by running ``testflinger config``. If no value is found on the command line, config file, or environment variable, then a safe default value will be used. + +To set a configuration value in the config file, you can edit it directly or use ``testflinger config key=value``. + +Testflinger Config Variables +---------------------------- + +The following config values are currently supported: + +* server - Testflinger Server URL to use + + * Config file key: ``server`` + * Environment variable: ``TESTFLINGER_SERVER`` + * Default: ``https://testflinger.canonical.com`` + +* error_threshold - warn the user of possible server/network problems after failing to read from the server after this many consecutive attempts + + * Config file key: ``error_threshold`` + * Environment variable: ``TESTFLINGER_ERROR_THRESHOLD`` + * Default: ``3`` + +* client_id - Client ID to use for APIs that require authorisation + + * Config file key: ``client_id`` + * Environment variable: ``TESTFLINGER_CLIENT_ID`` + * Default: ``None`` + +* secret_key - Secret key to use for APIs that require authorisation + + * Config file key: ``secret_key`` + * Environment variable: ``TESTFLINGER_SECRET_KEY`` + * Default: ``None`` diff --git a/docs/reference/index.rst b/docs/reference/index.rst index efc8047a..d82ce0b3 100644 --- a/docs/reference/index.rst +++ b/docs/reference/index.rst @@ -23,4 +23,12 @@ Working with Testflinger servers and agents testflinger-server-conf testflinger-agent-conf - test-phases \ No newline at end of file + test-phases + +Using the Command Line Interface +-------------------------------- + +.. toctree:: + :maxdepth: 1 + + cli-config \ No newline at end of file