Refactoring instance manager

Ericsson · Aug 7, 2018 · a1e660a · a1e660a
1 parent 60c490d
commit a1e660a
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 75 deletions.
diff --git a/libcodechecker/analyze/log_parser.py b/libcodechecker/analyze/log_parser.py
@@ -224,9 +224,9 @@ def remove_file_if_exists(filename):
         os.remove(filename)
 
 
-def parse_compile_commands_json(logfile, parseLogOptions):
+def parse_compile_commands_json(log_data, parseLogOptions):
     """
-    logfile: is a compile command json
+    log_data: content of a compile command json.
     """
 
     output_path = parseLogOptions.output_path
@@ -239,13 +239,11 @@ def parse_compile_commands_json(logfile, parseLogOptions):
     actions = []
     filtered_build_actions = {}
 
-    data = load_json_or_empty(logfile, {})
-
     compiler_includes = {}
     compiler_target = {}
 
     counter = 0
-    for entry in data:
+    for entry in log_data:
         sourcefile = entry['file']
 
         if not os.path.isabs(sourcefile):
@@ -364,17 +362,17 @@ def parse_log(logfilepath, parseLogOptions):
     """
     LOG.debug('Parsing log file: ' + logfilepath)
 
-    with open(logfilepath) as logfile:
-        try:
-            actions = parse_compile_commands_json(logfile, parseLogOptions)
-        except (ValueError, KeyError, TypeError) as ex:
-            if os.stat(logfilepath).st_size == 0:
-                LOG.error('The compile database is empty.')
-            else:
-                LOG.error('The compile database is not valid.')
-            LOG.debug(traceback.format_exc())
-            LOG.debug(ex)
-            sys.exit(1)
+    try:
+        data = load_json_or_empty(logfilepath, {})
+        actions = parse_compile_commands_json(data, parseLogOptions)
+    except (ValueError, KeyError, TypeError) as ex:
+        if os.stat(logfilepath).st_size == 0:
+            LOG.error('The compile database is empty.')
+        else:
+            LOG.error('The compile database is not valid.')
+        LOG.debug(traceback.format_exc())
+        LOG.debug(ex)
+        sys.exit(1)
 
     LOG.debug('Parsing log file done.')
 

diff --git a/libcodechecker/server/instance_manager.py b/libcodechecker/server/instance_manager.py
@@ -22,22 +22,22 @@
 from libcodechecker.util import load_json_or_empty
 
 
-def __getInstanceDescriptorPath(folder=None):
+def __get_instance_descriptor_path(folder=None):
     if not folder:
         folder = os.path.expanduser("~")
 
     return os.path.join(folder, ".codechecker.instances.json")
 
 
-def __makeInstanceDescriptorFile(folder=None):
-    descriptor = __getInstanceDescriptorPath(folder)
+def __make_instance_descriptor_file(folder=None):
+    descriptor = __get_instance_descriptor_path(folder)
     if not os.path.exists(descriptor):
         with open(descriptor, 'w') as f:
             json.dump([], f)
         os.chmod(descriptor, stat.S_IRUSR | stat.S_IWUSR)
 
 
-def __checkInstance(hostname, pid):
+def __check_instance(hostname, pid):
     """Check if the given process on the system is a valid, running CodeChecker
     for the current user."""
 
@@ -57,25 +57,27 @@ def __checkInstance(hostname, pid):
         return False
 
 
-def __rewriteInstanceFile(append, remove, folder=None):
-    """This helper method reads the user's instance descriptor and manages it
-    eliminating dead records, appending new ones and reserialising the file."""
+def __rewrite_instance_file(append, remove, folder=None):
+    """
+    This helper method reads the user's instance descriptor and manages it
+    eliminating dead records, appending new ones and re-serialising the file.
+    """
+    __make_instance_descriptor_file(folder)
 
-    __makeInstanceDescriptorFile(folder)
-    with open(__getInstanceDescriptorPath(folder), 'r+') as f:
-        portalocker.lock(f, portalocker.LOCK_EX)
+    append_pids = [i['pid'] for i in append]
 
-        # After reading, check every instance if they are still valid and
-        # make sure PID does not collide accidentally with the
-        # to-be-registered instances, if any exists in the append list as it
-        # would cause duplication.
-        #
-        # Also, we remove the records to the given PIDs, if any exists.
-        append_pids = [i['pid'] for i in append]
-        instances = [i for i in json.load(f)
-                     if i['pid'] not in append_pids and
-                     (i['hostname'] + ":" + str(i['pid'])) not in remove and
-                     __checkInstance(i['hostname'], i['pid'])]
+    # After reading, check every instance if they are still valid and
+    # make sure PID does not collide accidentally with the
+    # to-be-registered instances, if any exists in the append list as it
+    # would cause duplication.
+    #
+    # Also, we remove the records to the given PIDs, if any exists.
+    instances = [i for i in get_instances(folder)
+                 if i['pid'] not in append_pids and
+                 (i['hostname'] + ":" + str(i['pid'])) not in remove]
+
+    with open(__get_instance_descriptor_path(folder), 'w') as f:
+        portalocker.lock(f, portalocker.LOCK_EX)
 
         instances = instances + append
 
@@ -91,12 +93,12 @@ def register(pid, workspace, port, folder=None):
     descriptor.
     """
 
-    __rewriteInstanceFile([{"pid": pid,
-                            "hostname": socket.gethostname(),
-                            "workspace": workspace,
-                            "port": port}],
-                          [],
-                          folder)
+    __rewrite_instance_file([{"pid": pid,
+                              "hostname": socket.gethostname(),
+                              "workspace": workspace,
+                              "port": port}],
+                            [],
+                            folder)
 
 
 def unregister(pid, folder=None):
@@ -105,15 +107,17 @@ def unregister(pid, folder=None):
     descriptor.
     """
 
-    __rewriteInstanceFile([], [socket.gethostname() + ":" + str(pid)], folder)
+    __rewrite_instance_file([],
+                            [socket.gethostname() + ":" + str(pid)],
+                            folder)
 
 
 def get_instances(folder=None):
     """Returns the list of running servers for the current user."""
 
     # This method does NOT write the descriptor file.
 
-    descriptor = __getInstanceDescriptorPath(folder)
+    descriptor = __get_instance_descriptor_path(folder)
     instances = load_json_or_empty(descriptor, {}, lock=True)
 
-    return [i for i in instances if __checkInstance(i['hostname'], i['pid'])]
+    return [i for i in instances if __check_instance(i['hostname'], i['pid'])]
diff --git a/tests/unit/test_buildcmd_escaping.py b/tests/unit/test_buildcmd_escaping.py
@@ -74,18 +74,16 @@ def __get_cmp_json(self, buildcmd):
                        "command": buildcmd + " -c " + self.src_file_path,
                        "file": self.src_file_path}
 
-        compile_cmds = [compile_cmd]
-        return json.dumps(compile_cmds)
+        return [compile_cmd]
 
     def __get_comp_actions(self, compile_cmd):
         """
         Generate a compilation command json file and parse it
         to return the compilation actions.
         """
         comp_cmd_json = self.__get_cmp_json(compile_cmd)
-        with closing(StringIO(comp_cmd_json)) as text:
-            return log_parser.parse_compile_commands_json(text,
-                                                          ParseLogOptions())
+        return log_parser.parse_compile_commands_json(comp_cmd_json,
+                                                      ParseLogOptions())
 
     def test_buildmgr(self):
         """

diff --git a/tests/unit/test_log_parser.py b/tests/unit/test_log_parser.py
@@ -150,26 +150,26 @@ def test_omit_preproc(self):
         """
         Compiler preprocessor actions should be omitted.
         """
-        preprocessor_actions = StringIO('''[
+        preprocessor_actions = [
             {"directory": "/tmp",
-            "command": "g++ /tmp/a.cpp -c /tmp/a.cpp",
-            "file": "/tmp/a.cpp" },
+             "command": "g++ /tmp/a.cpp -c /tmp/a.cpp",
+             "file": "/tmp/a.cpp"},
             {"directory": "/tmp",
-            "command": "g++ /tmp/a.cpp -E /tmp/a.cpp",
-            "file": "/tmp/a.cpp" },
+             "command": "g++ /tmp/a.cpp -E /tmp/a.cpp",
+             "file": "/tmp/a.cpp"},
             {"directory": "/tmp",
-            "command": "g++ /tmp/a.cpp -MT /tmp/a.cpp",
-            "file": "/tmp/a.cpp" },
+             "command": "g++ /tmp/a.cpp -MT /tmp/a.cpp",
+             "file": "/tmp/a.cpp"},
             {"directory": "/tmp",
-            "command": "g++ /tmp/a.cpp -MM /tmp/a.cpp",
-            "file": "/tmp/a.cpp" },
+             "command": "g++ /tmp/a.cpp -MM /tmp/a.cpp",
+             "file": "/tmp/a.cpp"},
             {"directory": "/tmp",
-            "command": "g++ /tmp/a.cpp -MF /tmp/a.cpp",
-            "file": "/tmp/a.cpp" },
+             "command": "g++ /tmp/a.cpp -MF /tmp/a.cpp",
+             "file": "/tmp/a.cpp"},
             {"directory": "/tmp",
-            "command": "g++ /tmp/a.cpp -M /tmp/a.cpp",
-            "file": "/tmp/a.cpp" }]
-        ''')
+             "command": "g++ /tmp/a.cpp -M /tmp/a.cpp",
+             "file": "/tmp/a.cpp"}]
+
         build_actions = \
             log_parser.parse_compile_commands_json(preprocessor_actions,
                                                    ParseLogOptions())
@@ -182,11 +182,10 @@ def test_keep_compile_and_dep(self):
         """ Keep the compile command if -MD is set.
         Dependency generation is done as a side effect of the compilation.
         """
-        preprocessor_actions = StringIO('''[
+        preprocessor_actions = [
             {"directory": "/tmp",
-            "command": "g++ /tmp/a.cpp -MD /tmp/a.cpp",
-            "file": "/tmp/a.cpp" }]
-        ''')
+             "command": "g++ /tmp/a.cpp -MD /tmp/a.cpp",
+             "file": "/tmp/a.cpp"}]
 
         build_actions = \
             log_parser.parse_compile_commands_json(preprocessor_actions,
@@ -197,14 +196,13 @@ def test_keep_compile_and_dep(self):
     def test_omit_dep_with_e(self):
         """ Skip the compile command if -MD is set together with -E. """
 
-        preprocessor_actions = StringIO('''[
+        preprocessor_actions = [
             {"directory": "/tmp",
-            "command": "g++ /tmp/a.cpp -MD -E /tmp/a.cpp",
-            "file": "/tmp/a.cpp" },
+             "command": "g++ /tmp/a.cpp -MD -E /tmp/a.cpp",
+             "file": "/tmp/a.cpp"},
             {"directory": "/tmp",
-            "command": "g++ /tmp/a.cpp -E -MD /tmp/a.cpp",
-            "file": "/tmp/a.cpp" } ]
-        ''')
+             "command": "g++ /tmp/a.cpp -E -MD /tmp/a.cpp",
+             "file": "/tmp/a.cpp"}]
 
         build_actions = \
             log_parser.parse_compile_commands_json(preprocessor_actions,