Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading