Skip to content

Commit

Permalink
refactor(interactive): Add format check for Flex Interactive code (#4272
Browse files Browse the repository at this point in the history
)

Implement format and style checks for C++ and Python code in Flex
Interactive; Java checks are already in place.

Fix #4270
  • Loading branch information
zhanglei1949 authored Oct 8, 2024
1 parent cbceab0 commit e8acc7d
Show file tree
Hide file tree
Showing 22 changed files with 528 additions and 361 deletions.
38 changes: 37 additions & 1 deletion .github/workflows/pr-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions flex/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions flex/codegen/src/building_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down
1 change: 0 additions & 1 deletion flex/codegen/src/string_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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(" ") +
Expand Down
1 change: 0 additions & 1 deletion flex/engines/graph_db/database/graph_db_operations.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 0 additions & 1 deletion flex/engines/graph_db/runtime/common/operators/dedup.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ namespace gs {

namespace runtime {


class Dedup {
public:
static void dedup(const ReadTransaction& txn, Context& ctx,
Expand Down
4 changes: 1 addition & 3 deletions flex/engines/hqps_db/structures/collection.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down
47 changes: 26 additions & 21 deletions flex/interactive/sdk/examples/python/basic_example.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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",
Expand All @@ -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"}
],
Expand All @@ -112,6 +108,7 @@
],
}


def createGraph(sess: Session):
create_graph_request = CreateGraphRequest.from_dict(test_graph_def)
resp = sess.create_graph(create_graph_request)
Expand Down Expand Up @@ -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):
Expand All @@ -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):
Expand All @@ -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):
Expand Down Expand Up @@ -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):
Expand All @@ -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__":
Expand Down Expand Up @@ -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
Expand All @@ -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)
33 changes: 17 additions & 16 deletions flex/interactive/sdk/python/gs_interactive/client/driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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,
Expand All @@ -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:
Expand All @@ -71,7 +66,7 @@ def close(self):
"""
if self._neo4j_driver is not None:
self._neo4j_driver.close()

def __del__(self):
self.close()

Expand All @@ -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:
Expand All @@ -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)

Expand Down
8 changes: 4 additions & 4 deletions flex/interactive/sdk/python/gs_interactive/client/result.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down
Loading

0 comments on commit e8acc7d

Please sign in to comment.