From 6c86c6b75dbae75c42d233d8746656f262877c9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20Tomsa?= Date: Thu, 12 Dec 2024 16:54:18 +0100 Subject: [PATCH] feat: Improve test URLs output _test_urls and _legacy_test_urls output is nicer, with clear SUCCESS/FAILURE statement. URLs are consistently listed, so is legacy fallback. With --verbose turned on, more information about requests, responses and errors are printed. The readability of the output improved drastically, with only little changes to the logging and tiny touches to the logic. The generic HTTP method logs information about the request. To make the log messages blend nicely into the connection test, introduced logging-related arguments: * To keep the output concise by default, but more helpful with --verbose, log_level suppresses HTTP details. * To match indentation with messages outside the request method, log_prefix allows to add spaces to the beginning. --- insights/client/connection.py | 94 ++++++++++++++++++----------------- 1 file changed, 49 insertions(+), 45 deletions(-) diff --git a/insights/client/connection.py b/insights/client/connection.py index abfe0149ad..1664f442f9 100644 --- a/insights/client/connection.py +++ b/insights/client/connection.py @@ -168,7 +168,7 @@ def _init_session(self): session.trust_env = False return session - def _http_request(self, url, method, log_response_text=True, **kwargs): + def _http_request(self, url, method, log_response_text=True, log_prefix="", log_level=NETWORK, **kwargs): ''' Perform an HTTP request, net logging, and error handling Parameters @@ -178,7 +178,7 @@ def _http_request(self, url, method, log_response_text=True, **kwargs): Returns HTTP response object ''' - log_message = "{method} {url}".format(method=method, url=url) + log_message = "{log_prefix}{method} {url}".format(log_prefix=log_prefix, method=method, url=url) if "data" in kwargs.keys(): log_message += " data={data}".format(data=kwargs["data"]) if "json" in kwargs.keys(): @@ -193,14 +193,14 @@ def _http_request(self, url, method, log_response_text=True, **kwargs): else: attachments.append(name) log_message += " attachments={files}".format(files=",".join(attachments)) - logger.log(NETWORK, log_message) + logger.log(log_level, log_message) try: res = self.session.request(url=url, method=method, timeout=self.config.http_timeout, **kwargs) except Exception: raise - logger.log(NETWORK, "HTTP Status: %d %s", res.status_code, res.reason) + logger.log(log_level, "%sHTTP Status: %d %s", log_prefix, res.status_code, res.reason) if log_response_text or res.status_code // 100 != 2: - logger.log(NETWORK, "HTTP Response Text: %s", res.text) + logger.log(log_level, "%sHTTP Response Text: %s", log_prefix, res.text) return res def get(self, url, **kwargs): @@ -357,26 +357,24 @@ def _legacy_test_urls(self, url, method): test_url = url.scheme + "://" + url.netloc last_ex = None paths = (url.path + '/', '', '/r', '/r/insights') + log_level = NETWORK if self.config.verbose else logging.DEBUG for ext in paths: try: - logger.log(NETWORK, "Testing: %s", test_url + ext) + logger.info(" Testing %s", test_url + ext) if method == "POST": - test_req = self.post(test_url + ext, data=test_flag) + test_req = self.post(test_url + ext, data=test_flag, log_prefix=" ", log_level=log_level) elif method == "GET": - test_req = self.get(test_url + ext) + test_req = self.get(test_url + ext, log_prefix=" ", log_level=log_level) # Strata returns 405 on a GET sometimes, this isn't a big deal if test_req.status_code in (200, 201): - logger.info( - "Successfully connected to: %s", test_url + ext) return True else: - logger.info("Connection failed") + logger.error(" Failed.") return False except REQUEST_FAILED_EXCEPTIONS as exc: last_ex = exc - logger.error( - "Could not successfully connect to: %s", test_url + ext) - print(exc) + logger.debug(" Caught %s: %s", type(exc).__name__, exc) + logger.error(" Failed.") if last_ex: raise last_ex @@ -387,27 +385,26 @@ def _test_urls(self, url, method): if self.config.legacy_upload: return self._legacy_test_urls(url, method) try: - logger.log(NETWORK, 'Testing %s', url) + logger.info(' Testing %s', url) + + log_prefix = " " + log_level = NETWORK if self.config.verbose else logging.DEBUG + if method == 'POST': test_tar = TemporaryFile(mode='rb', suffix='.tar.gz') test_files = { 'file': ('test.tar.gz', test_tar, 'application/vnd.redhat.advisor.collection+tgz'), 'metadata': '{\"test\": \"test\"}' } - test_req = self.post(url, files=test_files) + test_req = self.post(url, files=test_files, log_prefix=log_prefix, log_level=log_level) elif method == "GET": - test_req = self.get(url) + test_req = self.get(url, log_prefix=log_prefix, log_level=log_level) if test_req.status_code in (200, 201, 202): - logger.info( - "Successfully connected to: %s", url) return True else: - logger.info("Connection failed") return False except REQUEST_FAILED_EXCEPTIONS as exc: - logger.error( - "Could not successfully connect to: %s", url) - print(exc) + logger.debug(" Caught %s: %s", type(exc).__name__, exc) raise def _test_auth_config(self): @@ -493,29 +490,36 @@ def test_connection(self, rc=0): self._dump_urls() logger.info("Running Connection Tests...") + logger.info("") - try: - logger.info("=== Begin Upload URL Connection Test ===") - upload_success = self._test_urls(self.upload_url, "POST") - logger.info("=== End Upload URL Connection Test: %s ===\n", - "SUCCESS" if upload_success else "FAILURE") - logger.info("=== Begin API URL Connection Test ===") - api_success = self._test_urls(self.ping_url, 'GET') - logger.info("=== End API URL Connection Test: %s ===\n", - "SUCCESS" if api_success else "FAILURE") - if upload_success and api_success: - logger.info("Connectivity tests completed successfully") - print("See %s for more details." % self.config.logging_file) - else: - logger.info("Connectivity tests completed with some errors") - print("See %s for more details." % self.config.logging_file) - rc = 1 - except REQUEST_FAILED_EXCEPTIONS: - logger.error('Connectivity test failed! ' - 'Please check your network configuration') - print('Additional information may be in %s' % self.config.logging_file) - return 1 - return rc + for description, url, method in [ + ("Uploading a file to Ingress", self.upload_url, "POST"), + ("Pinging the API", self.ping_url, "GET"), + ]: + logger.info(" %s...", description) + + try: + success = self._test_urls(url, method) + except REQUEST_FAILED_EXCEPTIONS as exc: + last_ex = exc + success = False + + if not success: + break + + logger.info(" SUCCESS.") + logger.info("") + else: + logger.info(" See %s for more details." % self.config.logging_file) + return rc + + logger.error(" FAILED.") + logger.error("") + logger.error(" Please check your network configuration") + logger.error(" Additional information may be in %s" % self.config.logging_file) + logger.error("") + + return 1 def handle_fail_rcs(self, req): """