refactor(interactive): Add format check for Flex Interactive code (#4272

) Implement format and style checks for C++ and Python code in Flex Interactive; Java checks are already in place. Fix #4270
alibaba · Oct 8, 2024 · e8acc7d · e8acc7d
1 parent cbceab0
commit e8acc7d
Show file tree

Hide file tree

Showing 22 changed files with 528 additions and 361 deletions.
diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml
@@ -109,9 +109,10 @@ jobs:
         sudo chmod +x /usr/bin/clang-format
 
         # run format
-        cd analytical_engine/
+        pushd analytical_engine/
         find ./apps ./benchmarks ./core ./frame ./misc ./test  -name "*.h" | xargs clang-format -i --style=file
         find ./apps ./benchmarks ./core ./frame ./misc ./test  -name "*.cc" | xargs clang-format -i --style=file
+        popd
 
         # validate format
         function prepend() { while read line; do echo "${1}${line}"; done; }
@@ -137,6 +138,33 @@ jobs:
             exit -1
         fi
 
+        push flex
+        # except for files end with act.h 
+        find ./bin ./codegen ./engines ./storages ./engines ./tests ./utils ./otel -name "*.h" ! -name "*act.h" ! -name "*actg.h" | xargs clang-format -i --style=file
+        find ./bin ./codegen ./engines ./storages ./engines ./tests ./utils ./otel -name "*.cc" ! -name "*act.cc" | xargs clang-format -i --style=file
+        popd
+
+        GIT_DIFF=$(git diff --ignore-submodules)
+        if [[ -n $GIT_DIFF ]]; then
+            echo "~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"
+            echo "| clang-format failures found!"
+            echo "|"
+            echo "$GIT_DIFF" | prepend "| "
+            echo "|"
+            echo "| Run: "
+            echo "|"
+            echo "|    cd flex && make flex_clformat"
+            echo "|"
+            echo "| to fix this error."
+            echo "|"
+            echo "| Ensure you are working with clang-format-8, which can be obtained from"
+            echo "|"
+            echo "|    https://github.com/muttleyxd/clang-tools-static-binaries/releases"
+            echo "|"
+            echo "~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"
+            exit -1
+        fi
+
     - name: Java Format and Lint Check
       run: |
         wget https://github.com/google/google-java-format/releases/download/v1.13.0/google-java-format-1.13.0-all-deps.jar
@@ -184,6 +212,14 @@ jobs:
         python3 -m isort --check --diff .
         python3 -m black --check --diff .
         python3 -m flake8 .
+        popd
+        pushd flex/interactive/sdk/python/gs_interactive
+        python3 -m isort --check --diff .
+        python3 -m black --check --diff .
+        # only check client and tests, to avoid checking generated code under api, client/generated, etc.
+        python3 -m flake8 ./client 
+        python3 -m flake8 ./tests 
+        popd
 
     - name: Generate Docs
       shell: bash

diff --git a/flex/CMakeLists.txt b/flex/CMakeLists.txt
@@ -249,13 +249,17 @@ install(EXPORT flex-targets
 file(GLOB_RECURSE FILES_NEED_LINT
         "engines/*.cc"
         "engines/*.h"
+        "codegen/*.cc"
+        "codegen/*.h"
         "bin/*.cc"
         "storages/*.h"
         "storages/*.cc"
         "otel/*.h"
         "otel/*.cc"
         "test/*.h"
         "test/*.cc"
+        "utils/*.h"
+        "utils/*.cc"
         EXCEPT "*.act.h" "*.actg.h" "*.autogen.h" "*.autogen.cc")
 list(FILTER FILES_NEED_LINT EXCLUDE REGEX ".*\.act.h$|.*\.actg.h$|.*\.autogen.h$|.*\.autogen.cc$")
 # gsa_clformat

diff --git a/flex/codegen/src/building_context.h b/flex/codegen/src/building_context.h
@@ -126,9 +126,7 @@ struct TagIndMapping {
            tag_id_2_tag_inds_[tag_id] != -1;
   }
 
-  int32_t GetMaxTagId() const {
-    return tag_id_2_tag_inds_.size() - 1;
-  }
+  int32_t GetMaxTagId() const { return tag_id_2_tag_inds_.size() - 1; }
 
   // convert tag_ind (us) to tag ids
   std::vector<int32_t> tag_ind_2_tag_ids_;

diff --git a/flex/codegen/src/string_utils.h b/flex/codegen/src/string_utils.h
@@ -82,7 +82,6 @@ std::string res_alias_to_append_opt(int res_alias, int in_alias) {
   }
 }
 
-
 template <typename LabelIdT>
 std::string ensure_label_id(LabelIdT label_id) {
   return std::string(LABEL_ID_T_CASTER) + std::string(" ") +

diff --git a/flex/engines/graph_db/database/graph_db_operations.h b/flex/engines/graph_db/database/graph_db_operations.h
@@ -19,7 +19,6 @@
 #include <unordered_map>
 #include <vector>
 
-
 #include "flex/engines/graph_db/database/graph_db.h"
 #include "flex/engines/graph_db/database/graph_db_session.h"
 #include "utils/result.h"

diff --git a/flex/engines/graph_db/runtime/common/operators/dedup.h b/flex/engines/graph_db/runtime/common/operators/dedup.h
@@ -26,7 +26,6 @@ namespace gs {
 
 namespace runtime {
 
-
 class Dedup {
  public:
   static void dedup(const ReadTransaction& txn, Context& ctx,

diff --git a/flex/engines/hqps_db/structures/collection.h b/flex/engines/hqps_db/structures/collection.h
@@ -434,9 +434,7 @@ class CountBuilder {
     return true;
   }
 
-  Collection<int64_t> Build() {
-    return Collection<int64_t>(std::move(vec_));
-  }
+  Collection<int64_t> Build() { return Collection<int64_t>(std::move(vec_)); }
 
  private:
   std::vector<int64_t> vec_;

diff --git a/flex/interactive/sdk/examples/python/basic_example.py b/flex/interactive/sdk/examples/python/basic_example.py
@@ -15,14 +15,17 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
-import time
 import argparse
 import os
+import time
+
 from gs_interactive.client.driver import Driver
 from gs_interactive.client.session import Session
 from gs_interactive.models import *
 
-MODERN_GRAPH_CSV_DIR=os.path.join(os.path.dirname(__file__), "../../../../interactive/examples/modern_graph")
+MODERN_GRAPH_CSV_DIR = os.path.join(
+    os.path.dirname(__file__), "../../../../interactive/examples/modern_graph"
+)
 # get current dir
 
 test_graph_def = {
@@ -72,12 +75,7 @@
 }
 
 test_graph_datasource = {
-    "loading_config": {
-        "data_source" : {
-            "scheme": "file"
-        },
-        "import_option" : "init"
-    },
+    "loading_config": {"data_source": {"scheme": "file"}, "import_option": "init"},
     "vertex_mappings": [
         {
             "type_name": "person",
@@ -96,9 +94,7 @@
                 "source_vertex": "person",
                 "destination_vertex": "person",
             },
-            "inputs": [
-                f"@{MODERN_GRAPH_CSV_DIR}/person_knows_person.csv"
-            ],
+            "inputs": [f"@{MODERN_GRAPH_CSV_DIR}/person_knows_person.csv"],
             "source_vertex_mappings": [
                 {"column": {"index": 0, "name": "person.id"}, "property": "id"}
             ],
@@ -112,6 +108,7 @@
     ],
 }
 
+
 def createGraph(sess: Session):
     create_graph_request = CreateGraphRequest.from_dict(test_graph_def)
     resp = sess.create_graph(create_graph_request)
@@ -169,7 +166,9 @@ def addVertex(sess: Session, graph_id: str):
     if api_response.is_ok():
         print("The response of add_vertex:\n", api_response)
     else:
-        raise Exception("add_vertex failed with error: %s" % api_response.get_status_message())
+        raise Exception(
+            "add_vertex failed with error: %s" % api_response.get_status_message()
+        )
 
 
 def updateVertex(sess: Session, graph_id: str):
@@ -182,7 +181,9 @@ def updateVertex(sess: Session, graph_id: str):
     if api_response.is_ok():
         print("The response of update_vertex", api_response)
     else:
-        raise Exception("update_vertex failed with error: %s" % api_response.get_status_message())
+        raise Exception(
+            "update_vertex failed with error: %s" % api_response.get_status_message()
+        )
 
 
 def getVertex(sess: Session, graph_id: str):
@@ -192,7 +193,9 @@ def getVertex(sess: Session, graph_id: str):
     if api_response.is_ok():
         print("The response of get_vertex", api_response)
     else:
-        raise Exception("get_vertex failed with error: %s" % api_response.get_status_message())
+        raise Exception(
+            "get_vertex failed with error: %s" % api_response.get_status_message()
+        )
 
 
 def updateEdge(sess: Session, graph_id: str):
@@ -230,8 +233,9 @@ def getEdge(sess: Session, graph_id: str):
     if api_response.is_ok():
         print("The response of get_edge", api_response)
     else:
-        raise Exception("get_edge failed with error: %s" % api_response.get_status_message())
-
+        raise Exception(
+            "get_edge failed with error: %s" % api_response.get_status_message()
+        )
 
 
 def addEdge(sess: Session, graph_id: str):
@@ -257,8 +261,9 @@ def addEdge(sess: Session, graph_id: str):
     if api_response.is_ok():
         print("The response of add_edge", api_response)
     else:
-        raise Exception("add_edge failed with error: %s" % api_response.get_status_message())
-
+        raise Exception(
+            "add_edge failed with error: %s" % api_response.get_status_message()
+        )
 
 
 if __name__ == "__main__":
@@ -299,7 +304,7 @@ def addEdge(sess: Session, graph_id: str):
         )
         resp = sess.create_procedure(graph_id, create_proc_request)
         assert resp.is_ok()
-        
+
         get_proc_res = sess.get_procedure(graph_id, "test_procedure")
         assert get_proc_res.is_ok()
         # Check the description of the procedure
@@ -316,11 +321,11 @@ def addEdge(sess: Session, graph_id: str):
             result = session.run("CALL test_procedure();")
             for record in result:
                 print(record)
-                
+
         addVertex(sess, graph_id)
         getVertex(sess, graph_id)
         updateVertex(sess, graph_id)
-        
+
         addEdge(sess, graph_id)
         getEdge(sess, graph_id)
         updateEdge(sess, graph_id)
diff --git a/flex/interactive/sdk/python/gs_interactive/client/driver.py b/flex/interactive/sdk/python/gs_interactive/client/driver.py
@@ -17,14 +17,8 @@
 #
 
 import os
-import sys
 
-from gremlin_python import statics
 from gremlin_python.driver.client import Client
-from gremlin_python.driver.driver_remote_connection import DriverRemoteConnection
-from gremlin_python.process.graph_traversal import __
-from gremlin_python.process.strategies import *
-from gremlin_python.structure.graph import Graph
 from neo4j import GraphDatabase
 from neo4j import Session as Neo4jSession
 
@@ -33,10 +27,11 @@
 
 class Driver:
     """
-    The main entry point for the Interactive SDK. With the Interactive endpoints provided, you can create a Interactive Session to interact with the Interactive service,
+    The main entry point for the Interactive SDK. With the Interactive endpoints provided,
+    you can create a Interactive Session to interact with the Interactive service,
     and create a Neo4j Session to interact with the Neo4j service.
     """
-    
+
     def __init__(
         self,
         admin_endpoint: str = None,
@@ -45,8 +40,8 @@ def __init__(
         gremlin_endpoint: str = None,
     ):
         """
-        Construct a new driver using the specified endpoints. 
-        If no endpoints are provided, the driver will read them from environment variables. 
+        Construct a new driver using the specified endpoints.
+        If no endpoints are provided, the driver will read them from environment variables.
         You will receive the endpoints after starting the Interactive service.
 
         Args:
@@ -71,7 +66,7 @@ def close(self):
         """
         if self._neo4j_driver is not None:
             self._neo4j_driver.close()
-    
+
     def __del__(self):
         self.close()
 
@@ -96,21 +91,26 @@ def read_endpoints_from_env(self):
         self._admin_endpoint = os.environ.get("INTERACTIVE_ADMIN_ENDPOINT")
         assert (
             self._admin_endpoint is not None
-        ), "INTERACTIVE_ADMIN_ENDPOINT is not set, did you forget to export the environment variable after deploying Interactive? see https://graphscope.io/docs/latest/flex/interactive/installation"
+        ), "INTERACTIVE_ADMIN_ENDPOINT is not set, "
+        "did you forget to export the environment variable after deploying Interactive?"
+        " see https://graphscope.io/docs/latest/flex/interactive/installation"
         self._stored_proc_endpoint = os.environ.get("INTERACTIVE_STORED_PROC_ENDPOINT")
         if self._stored_proc_endpoint is None:
             print(
-                "INTERACTIVE_STORED_PROC_ENDPOINT is not set, will try to get it from service status endpoint"
+                "INTERACTIVE_STORED_PROC_ENDPOINT is not set,"
+                "will try to get it from service status endpoint"
             )
         self._cypher_endpoint = os.environ.get("INTERACTIVE_CYPHER_ENDPOINT")
         if self._cypher_endpoint is None:
             print(
-                "INTERACTIVE_CYPHER_ENDPOINT is not set, will try to get it from service status endpoint"
+                "INTERACTIVE_CYPHER_ENDPOINT is not set,"
+                "will try to get it from service status endpoint"
             )
         self._gremlin_endpoint = os.environ.get("INTERACTIVE_GREMLIN_ENDPOINT")
         if self._gremlin_endpoint is None:
             print(
-                "INTERACTIVE_GREMLIN_ENDPOINT is not set, will try to get it from service status endpoint"
+                "INTERACTIVE_GREMLIN_ENDPOINT is not set,"
+                "will try to get it from service status endpoint"
             )
 
     def session(self) -> Session:
@@ -133,7 +133,8 @@ def getNeo4jSession(self, **config) -> Neo4jSession:
         """
         Create a neo4j session with the specified endpoints.
         Args:
-            config: a dictionary of configuration options. The same as the ones in neo4j.Driver.session
+            config: a dictionary of configuration options, the same as the ones
+                in neo4j.Driver.session
         """
         return self.getNeo4jSessionImpl(**config)
 

diff --git a/flex/interactive/sdk/python/gs_interactive/client/result.py b/flex/interactive/sdk/python/gs_interactive/client/result.py
@@ -18,8 +18,6 @@
 
 from typing import Generic, TypeVar
 
-from pydantic import Field
-
 from gs_interactive.api_response import ApiResponse
 from gs_interactive.client.status import Status
 from gs_interactive.exceptions import ApiException
@@ -30,12 +28,14 @@
 
 class Result(Generic[T]):
     """
-    This is a generic class that wraps the result of an operation. It contains the status of the operation and the value returned by the operation.
+    This is a generic class that wraps the result of an operation,
+    It contains the status of the operation and the value returned by the operation.
     """
+
     def __init__(self, status: Status, value: T):
         """
         Construct a new Result object with the specified status and value.
-        
+
         Args:
             status: the status of the operation.
             value: the value returned by the operation.
-Original file line number
+Diff line change
@@ Expand Up @@
       }
     }
     template <typename LabelIdT>
     std::string ensure_label_id(LabelIdT label_id) {
       return std::string(LABEL_ID_T_CASTER) + std::string(" ") +
@@ Expand Down @@