Improved error reply on ssh check and module checks (#410)

filipecosta90 · web-flow · commit 3d424353e966 · 2023-03-28T18:25:40.000+01:00
* Improved error reply on ssh check and module checks

* flake8 fixes

* Fixed remote envs timeout checks
diff --git a/redisbench_admin/profilers/daemon.py b/redisbench_admin/profilers/daemon.py
@@ -282,7 +282,6 @@ def profile_stop(profiler_name, pid):
 
 
 def main():
-
     _, project_description, project_version = populate_with_poetry_data()
     project_name = "perf-daemon"
     parser = argparse.ArgumentParser(
diff --git a/redisbench_admin/profilers/perf_daemon_caller.py b/redisbench_admin/profilers/perf_daemon_caller.py
@@ -128,5 +128,4 @@ def stop_profile(self, **kwargs):
         return result
 
     def generate_outputs(self, use_case, **kwargs):
-
         return self.result, self.outputs, self.tabular_data_map
diff --git a/redisbench_admin/profilers/profilers_local.py b/redisbench_admin/profilers/profilers_local.py
@@ -259,7 +259,6 @@ def get_profilers_map(profilers_list, total_involved_processes, max_profilers=1)
                         perf = Perf()
                         profilers_map[profiler_name].append(perf)
             if "vtune" in profiler_name:
-
                 for profilers_per_type in range(total_involved_processes):
                     logging.info(
                         "Preparing Intel(R) VTune(TM) profiler for proc {} of {}".format(
diff --git a/redisbench_admin/run/common.py b/redisbench_admin/run/common.py
@@ -560,7 +560,6 @@ def run_redis_pre_steps(benchmark_config, r, required_modules):
         )
         search_specific_init(r, module_names)
     if required_modules is not None and len(required_modules) > 0:
-
         check_required_modules(module_names, required_modules)
 
         version = artifact_versions[0]
@@ -611,7 +610,6 @@ def dso_check(dso, local_module_file):
     if dso is None:
         logging.warning("No dso specified for perf analysis {}".format(dso))
         if local_module_file is not None:
-
             if type(local_module_file) == str:
                 dso = local_module_file
                 logging.warning(
diff --git a/redisbench_admin/run/modules.py b/redisbench_admin/run/modules.py
@@ -23,6 +23,7 @@ def redis_modules_check(local_module_files):
                     local_module_file
                 )
                 logging.error(error_message)
+                status = False
             else:
                 logging.info(
                     "Confirmed that module artifact: '{}' exists!".format(
diff --git a/redisbench_admin/run/redistimeseries.py b/redisbench_admin/run/redistimeseries.py
@@ -304,7 +304,6 @@ def timeseries_test_sucess_flow(
                 branch_target_table_keyname,
                 branch_target_table_dict,
             ) in branch_target_tables.items():
-
                 logging.info(
                     "Setting target table by branch on key {}".format(
                         branch_target_table_keyname
diff --git a/redisbench_admin/run/ssh.py b/redisbench_admin/run/ssh.py
@@ -72,7 +72,11 @@ def check_connection(ssh_conn):
 def ssh_pem_check(EC2_PRIVATE_PEM, private_key):
     if os.path.exists(private_key) is False:
         if EC2_PRIVATE_PEM is not None and EC2_PRIVATE_PEM != "":
-            logging.info("Given env variable EC2_PRIVATE_PEM exists")
+            logging.info(
+                "Given env variable EC2_PRIVATE_PEM exists saving it into {}".format(
+                    private_key
+                )
+            )
             with open(private_key, "w") as tmp_private_key_file:
                 pem_str = check_and_fix_pem_str(EC2_PRIVATE_PEM)
                 tmp_private_key_file.write(pem_str)
diff --git a/redisbench_admin/run_local/local_db.py b/redisbench_admin/run_local/local_db.py
@@ -136,7 +136,6 @@ def local_db_spin(
                 raise Exception("Redis process is not alive. Failing test.")
 
     if setup_type == "oss-cluster":
-
         cluster_init_steps(clusterconfig, redis_conns, local_module_file)
 
     if check_dbconfig_tool_requirement(benchmark_config):
diff --git a/redisbench_admin/run_remote/notifications.py b/redisbench_admin/run_remote/notifications.py
@@ -17,7 +17,6 @@ def generate_failure_notification(
     branch=None,
     tag=None,
 ):
-
     headline_test = "❌ FAILED CI job named {} in repo {}/{} due to {}".format(
         job_name, gh_org, gh_repo, failure_reason
     )
@@ -45,7 +44,6 @@ def generate_new_pr_comment_notification(
     regression_count,
     action,
 ):
-
     headline_test = "👨‍💻 {} PR Performance comment in repo {}/{}\n".format(
         action, gh_org, gh_repo
     )
diff --git a/redisbench_admin/run_remote/run_remote.py b/redisbench_admin/run_remote/run_remote.py
@@ -379,7 +379,6 @@ def run_remote_command_logic(args, project_name, project_version):
                             s3_bucket_name, test_name, tf_github_org, tf_github_repo
                         )
                         if setup_type in args.allowed_envs:
-
                             logging.info(
                                 "Starting setup named {} of topology type {}. Total primaries: {}".format(
                                     setup_name, setup_type, shard_count
@@ -422,7 +421,6 @@ def run_remote_command_logic(args, project_name, project_version):
                                 # after we've created the env, even on error we should always teardown
                                 # in case of some unexpected error we fail the test
                                 try:
-
                                     (
                                         _,
                                         _,
diff --git a/redisbench_admin/run_remote/standalone.py b/redisbench_admin/run_remote/standalone.py
@@ -27,7 +27,6 @@ def spin_up_standalone_remote_redis(
     modules_configuration_parameters_map={},
     redis_7=True,
 ):
-
     full_logfile, initial_redis_cmd = generate_remote_standalone_redis_cmd(
         logfile,
         redis_configuration_parameters,
@@ -77,7 +76,6 @@ def remote_module_files_cp(
     remote_module_files = []
     if local_module_files is not None:
         for local_module_file in local_module_files:
-
             splitted_module_and_plugins = local_module_file.split(" ")
             if len(splitted_module_and_plugins) > 1:
                 logging.info(
@@ -117,6 +115,11 @@ def remote_module_files_cp(
                     remote_module_files_in = remote_module_files_in + " "
                 remote_module_files_in = remote_module_files_in + remote_module_file
         remote_module_files.append(remote_module_files_in)
+    logging.info(
+        "There are a total of {} remote files {}".format(
+            len(remote_module_files), remote_module_files
+        )
+    )
     return remote_module_files
 
 
@@ -151,6 +154,9 @@ def generate_remote_standalone_redis_cmd(
                 command, remote_module_files, modules_configuration_parameters_map
             )
         if type(remote_module_files) == list:
+            logging.info(
+                "There are a total of {} modules".format(len(remote_module_files))
+            )
             for mod in remote_module_files:
                 redis_server_config_module_part(
                     command, mod, modules_configuration_parameters_map
diff --git a/redisbench_admin/utils/benchmark_config.py b/redisbench_admin/utils/benchmark_config.py
@@ -344,7 +344,6 @@ def extract_benchmark_type_from_config(
     benchmark_config_present = False
     benchmark_type = None
     if config_key in benchmark_config:
-
         if type(benchmark_config[config_key]) == list:
             for entry in benchmark_config[config_key]:
                 if benchmark_type_key in entry:
diff --git a/redisbench_admin/utils/remote.py b/redisbench_admin/utils/remote.py
@@ -725,7 +725,6 @@ def common_timeseries_extraction(
     target_tables = {}
     cleaned_metrics_arr = extract_results_table(metrics, results_dict)
     for cleaned_metric in cleaned_metrics_arr:
-
         metric_jsonpath = cleaned_metric[0]
         metric_context_path = cleaned_metric[1]
         metric_name = cleaned_metric[2]
diff --git a/tests/test_benchmark_config.py b/tests/test_benchmark_config.py
@@ -231,7 +231,10 @@ def test_process_benchmark_definitions_remote_timeouts():
     )
     assert len(remote_envs_timeout.keys()) == 2
     # we have the default timeout + the one specified
-    assert list(remote_envs_timeout.values())[0] == 600 + 1200
+    timeouts = list(remote_envs_timeout.values())
+    timeouts.sort()
+    assert (600 + 1200) in timeouts
+    assert (3600 + 600 + 1200) in timeouts
 
 
 def test_get_testfiles_to_process():
diff --git a/tests/test_compare.py b/tests/test_compare.py
@@ -10,7 +10,6 @@
 
 
 def test_compare_command_logic():
-
     rts_host = os.getenv("RTS_DATASINK_HOST", None)
     rts_port = 16379
     rts_pass = ""
diff --git a/tests/test_run_local.py b/tests/test_run_local.py
@@ -119,11 +119,12 @@ def test_run_local_command_logic_redis_benchmark():
         run_local_command_logic(args, "tool", "v0")
     except SystemExit as e:
         assert e.code == 0
-
-    r = redis.Redis()
-    total_keys = r.info("keyspace")["db0"]["keys"]
-    r.shutdown(nosave=True)
-    assert total_keys == 1000
+    finally:
+        r = redis.Redis()
+        r.ping()
+        total_keys = r.info("keyspace")["db0"]["keys"]
+        r.shutdown(nosave=True)
+        assert total_keys == 1000
 
 
 def test_run_local_command_logic():
diff --git a/tests/test_standalone.py b/tests/test_standalone.py
@@ -73,3 +73,15 @@ def test_generate_remote_standalone_redis_cmd():
         "log1", None, ["m1.so"], ".", modules_configuration_parameters_map, False
     )
     assert initial_redis_cmd.endswith("m1.so CHUNK_SIZE_BYTES 128")
+
+    # 2 modules
+    modules_configuration_parameters_map = {"m1": {"CHUNK_SIZE_BYTES": 128}}
+    full_logfile, initial_redis_cmd = generate_remote_standalone_redis_cmd(
+        "log1",
+        None,
+        ["m1.so", "m2.so"],
+        ".",
+        modules_configuration_parameters_map,
+        False,
+    )
+    assert initial_redis_cmd.endswith("m2.so")

Original file line number	Diff line number	Diff line change
`@@ -23,6 +23,7 @@ def redis_modules_check(local_module_files):`
`23`	`23`	`local_module_file`
`24`	`24`	`)`
`25`	`25`	`logging.error(error_message)`
	`26`	`+ status = False`
`26`	`27`	`else:`
`27`	`28`	`logging.info(`
`28`	`29`	`"Confirmed that module artifact: '{}' exists!".format(`