Merge pull request #1 from redhat-performance/fix1

Fixing some more linter issues

Containerfile

@@ -1,5 +1,5 @@
 FROM registry.fedoraproject.org/fedora:36
-MAINTAINER "Jan Hutar" <[email protected]>
+LABEL maintainer="[email protected]"
 
 RUN dnf -y install python3-boto3 python3-virtualenv python3-pip python3-psycopg2 python3-requests python3-psutil python3-pyyaml postgresql git-core dumb-init 'dnf-command(builddep)' \
     && dnf -y builddep python3-requests python3-psutil \

core/opl/status_data_updater.py

@@ -120,11 +120,7 @@ def doit_list(args):
     table = []
 
     for item in response["hits"]["hits"]:
-        logging.debug(
-            f"Loading data from document ID {item['_id']} with field id={item['_source']['id'] if 'id' in item['_source'] else None}"
-        )
-        tmpfile = tempfile.NamedTemporaryFile(prefix=item["_id"], delete=False).name
-        sd = opl.status_data.StatusData(tmpfile, data=item["_source"])
+        sd = _create_sd_from_es_response(item)
         row = [
             sd.get("id"),
             sd.get("started"),
@@ -146,11 +142,7 @@ def doit_change(args):
     source = response["hits"]["hits"][0]
     es_type = source["_type"]
     es_id = source["_id"]
-    logging.debug(
-        f"Loading data from document ID {source['_id']} with field id={source['_source']['id']}"
-    )
-    tmpfile = tempfile.NamedTemporaryFile(prefix=source["_id"], delete=False).name
-    sd = opl.status_data.StatusData(tmpfile, data=source["_source"])
+    sd = _create_sd_from_es_response(source)
 
     for item in args.change_set:
         if item == "":
@@ -274,6 +266,15 @@ def _get_rp_launch_results(session, args, launch):
     return results
 
 
+def _create_sd_from_es_response(response):
+    """Convert ElasticSearch response data structure to StatusData object."""
+    logging.debug(
+        f"Loading data from document ID {response['_id']} with field id={response['_source']['id'] if 'id' in response['_source'] else None}"
+    )
+    tmpfile = tempfile.NamedTemporaryFile(prefix=response["_id"], delete=False).name
+    return opl.status_data.StatusData(tmpfile, data=response["_source"])
+
+
 def _get_es_result_for_rp_result(session, args, run_id, result):
     if args.rp_project == "satcpt":
         # OK, I agree we need a better way here.
@@ -303,11 +304,7 @@ def _get_es_result_for_rp_result(session, args, run_id, result):
         raise Exception(f"Failed to find test result in ES for {run_id}")
     es_type = source["_type"]
     es_id = source["_id"]
-    logging.debug(
-        f"Loading data from document ID {source['_id']} with field id={source['_source']['id']}"
-    )
-    tmpfile = tempfile.NamedTemporaryFile(prefix=source["_id"], delete=False).name
-    sd = opl.status_data.StatusData(tmpfile, data=source["_source"])
+    sd = _create_sd_from_es_response(source)
     return (sd, es_type, es_id)
 
 

opl/status_data_updater.py

@@ -120,11 +120,7 @@ def doit_list(args):
     table = []
 
     for item in response["hits"]["hits"]:
-        logging.debug(
-            f"Loading data from document ID {item['_id']} with field id={item['_source']['id'] if 'id' in item['_source'] else None}"
-        )
-        tmpfile = tempfile.NamedTemporaryFile(prefix=item["_id"], delete=False).name
-        sd = opl.status_data.StatusData(tmpfile, data=item["_source"])
+        sd = _create_sd_from_es_response(item)
         row = [
             sd.get("id"),
             sd.get("started"),
@@ -146,11 +142,7 @@ def doit_change(args):
     source = response["hits"]["hits"][0]
     es_type = source["_type"]
     es_id = source["_id"]
-    logging.debug(
-        f"Loading data from document ID {source['_id']} with field id={source['_source']['id']}"
-    )
-    tmpfile = tempfile.NamedTemporaryFile(prefix=source["_id"], delete=False).name
-    sd = opl.status_data.StatusData(tmpfile, data=source["_source"])
+    sd = _create_sd_from_es_response(source)
 
     for item in args.change_set:
         if item == "":
@@ -274,6 +266,15 @@ def _get_rp_launch_results(session, args, launch):
     return results
 
 
+def _create_sd_from_es_response(response):
+    """Convert ElasticSearch response data structure to StatusData object."""
+    logging.debug(
+        f"Loading data from document ID {response['_id']} with field id={response['_source']['id'] if 'id' in response['_source'] else None}"
+    )
+    tmpfile = tempfile.NamedTemporaryFile(prefix=response["_id"], delete=False).name
+    return opl.status_data.StatusData(tmpfile, data=response["_source"])
+
+
 def _get_es_result_for_rp_result(session, args, run_id, result):
     if args.rp_project == "satcpt":
         # OK, I agree we need a better way here.
@@ -303,11 +304,7 @@ def _get_es_result_for_rp_result(session, args, run_id, result):
         raise Exception(f"Failed to find test result in ES for {run_id}")
     es_type = source["_type"]
     es_id = source["_id"]
-    logging.debug(
-        f"Loading data from document ID {source['_id']} with field id={source['_source']['id']}"
-    )
-    tmpfile = tempfile.NamedTemporaryFile(prefix=source["_id"], delete=False).name
-    sd = opl.status_data.StatusData(tmpfile, data=source["_source"])
+    sd = _create_sd_from_es_response(source)
     return (sd, es_type, es_id)
 
 

tests/test_cluster_read.py

@@ -130,30 +130,20 @@ def test_copy_from(self):
         string = """
             - name: somevalue
               constant: Hello world
-            - name: mycopyfrom
+            - name: mycopyfrom_exists
               copy_from: somevalue
+            - name: mycopyfrom_missing
+              copy_from: somevalue_that_does_not_exist
         """
         ri = opl.cluster_read.RequestedInfo(string)
         k, v = next(ri)
         self.assertEqual(k, "somevalue")
         self.assertEqual(v, "Hello world")
         k, v = next(ri)
-        self.assertEqual(k, "mycopyfrom")
-        self.assertEqual(v, "Hello world")
-
-    def test_copy_from_negative(self):
-        string = """
-            - name: somevalue
-              constant: Hello world
-            - name: mycopyfrom
-              copy_from: somevalue_that_does_not_exist
-        """
-        ri = opl.cluster_read.RequestedInfo(string)
-        k, v = next(ri)
-        self.assertEqual(k, "somevalue")
+        self.assertEqual(k, "mycopyfrom_exists")
         self.assertEqual(v, "Hello world")
         k, v = next(ri)
-        self.assertEqual(k, "mycopyfrom")
+        self.assertEqual(k, "mycopyfrom_missing")
         self.assertEqual(v, None)
 
     def test_wrong_config(self):

tests/test_status_data.py

@@ -86,7 +86,7 @@ def test_datetime(self):
         self.assertEqual(self.status_data.get_date("aaa"), now_plus2)
         self.assertEqual(self.status_data.get_date("bbb"), now_utc)
 
-    def test_datetime(self):
+    def test_datetime_format(self):
         refference = datetime.datetime(
             2020,
             12,
@@ -210,21 +210,21 @@ def test_set_subtree_json(self):
         with tempfile.NamedTemporaryFile(delete=False, suffix=".json") as f:
             f_name = f.name
             f.write(b'{"hello":"world","foo":42,"bar":{"baz":1}}')
-        self.status_data.set_subtree_json("results.xxx", f_name)
+        self.status_data.set_subtree_json("results.xxx_json", f_name)
         os.unlink(f_name)
-        self.assertEqual(self.status_data.get("results.xxx.hello"), "world")
-        self.assertEqual(self.status_data.get("results.xxx.foo"), 42)
-        self.assertEqual(self.status_data.get("results.xxx.bar.baz"), 1)
+        self.assertEqual(self.status_data.get("results.xxx_json.hello"), "world")
+        self.assertEqual(self.status_data.get("results.xxx_json.foo"), 42)
+        self.assertEqual(self.status_data.get("results.xxx_json.bar.baz"), 1)
 
     def test_set_subtree_yaml(self):
         with tempfile.NamedTemporaryFile(delete=False, suffix=".yaml") as f:
             f_name = f.name
             f.write(b"hello: world\nfoo: 42\nbar:\n  baz: 1")
-        self.status_data.set_subtree_json("results.xxx", f_name)
+        self.status_data.set_subtree_json("results.xxx_yaml", f_name)
         os.unlink(f_name)
-        self.assertEqual(self.status_data.get("results.xxx.hello"), "world")
-        self.assertEqual(self.status_data.get("results.xxx.foo"), 42)
-        self.assertEqual(self.status_data.get("results.xxx.bar.baz"), 1)
+        self.assertEqual(self.status_data.get("results.xxx_yaml.hello"), "world")
+        self.assertEqual(self.status_data.get("results.xxx_yaml.foo"), 42)
+        self.assertEqual(self.status_data.get("results.xxx_yaml.bar.baz"), 1)
 
     def test_remove_simple(self):
         self.status_data.set("results.xxx", "should not be here")
@@ -239,7 +239,7 @@ def test_remove_missing(self):
 
     def test_file_on_http(self):
         with self.assertRaises(requests.exceptions.ConnectionError):
-            status_data = opl.status_data.StatusData(
+            _ = opl.status_data.StatusData(
                 "http://does.not.exist/status-data-file.json"
             )