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

Support cypher query in python #2952

Merged
merged 16 commits into from
Jul 6, 2023
Merged

Support cypher query in python #2952

merged 16 commits into from
Jul 6, 2023

Conversation

andydiwenzhu
Copy link
Collaborator

What do these changes do?

Related issue number

Fixes #2736

@andydiwenzhu andydiwenzhu changed the title Support cypher query in python [WIP] Support cypher query in python Jun 30, 2023
@@ -232,7 +232,8 @@ def create_interactive_instance(
str(num_workers), # server size
str(self._interactive_port), # executor port
str(self._interactive_port + 1), # executor rpc port
str(self._interactive_port + 2 * num_workers), # frontend port
str(self._interactive_port + 2 * num_workers), # frontend gremlin port
str(self._interactive_port + 2 * num_workers + 1), # frontend cypher port
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try launch multiple interactive instance, and see if the port will conflict?
I mean, the line 240 may need to change.

return True

with GraphDatabase.driver(f'neo4j://{endpoint}', auth=("", "")) as driver:
_, _, _ = driver.execute_query(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the server is not ready, will this line raise exception?

fi
wait_period_seconds=$(($wait_period_seconds+5))
if [ ${wait_period_seconds} -gt ${timeout_seconds} ];then
echo "Get cypher external ip of ${GREMLIN_EXPOSE} failed."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider extract this logic into a function, since they are so similar, to alleviate the maintenance overhead.

@@ -42,7 +42,7 @@
from graphscope.client.session import get_default_session
from graphscope.client.session import get_option
from graphscope.client.session import graphlearn
from graphscope.client.session import gremlin
from graphscope.client.session import interactive
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't delete gremlin, make it a proxy to interactive

@@ -27,6 +27,8 @@
from graphscope.framework.dag import DAGNode
from graphscope.framework.dag_utils import gremlin_to_subgraph

from neo4j import GraphDatabase
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add try except, user may not want to install neo4j if they just need gremlin.
try:
from neo4j ...
except:
pass

cypher query is disabled

Committed-by: Diwen Zhu from Dev container

Committed-by: Diwen Zhu from Dev container

Committed-by: Diwen Zhu from Dev container
Committed-by: Diwen Zhu from Dev container

Committed-by: Diwen Zhu from Dev container
Committed-by: Diwen Zhu from Dev container

Committed-by: Diwen Zhu from Dev container
Committed-by: Diwen Zhu from Dev container

Committed-by: Diwen Zhu from Dev container
Committed-by: Diwen Zhu from Dev container

Committed-by: Diwen Zhu from Dev container

Committed-by: Diwen Zhu from Dev container

Committed-by: Diwen Zhu from Dev container
Committed-by: Diwen Zhu from Dev container

Committed-by: Diwen Zhu from Dev container
Committed-by: Diwen Zhu from Dev container

Committed-by: Diwen Zhu from Dev container
Committed-by: Diwen Zhu from Dev container

Committed-by: Diwen Zhu from Dev container
Committed-by: Diwen Zhu from Dev container
Committed-by: Diwen Zhu from Dev container
@codecov-commenter
Copy link

codecov-commenter commented Jul 4, 2023

Codecov Report

Merging #2952 (b93082a) into main (d12cd9a) will increase coverage by 30.91%.
The diff coverage is 79.24%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #2952       +/-   ##
===========================================
+ Coverage   42.37%   73.28%   +30.91%     
===========================================
  Files          99       99               
  Lines       10649    10687       +38     
===========================================
+ Hits         4512     7832     +3320     
+ Misses       6137     2855     -3282     
Impacted Files Coverage Δ
python/graphscope/client/session.py 75.85% <71.42%> (+9.68%) ⬆️
python/graphscope/interactive/query.py 83.15% <76.92%> (-4.91%) ⬇️
python/graphscope/__init__.py 83.33% <100.00%> (+0.47%) ⬆️
python/graphscope/client/rpc.py 82.48% <100.00%> (+7.29%) ⬆️
python/graphscope/tests/unittest/test_lazy.py 96.32% <100.00%> (+96.32%) ⬆️

... and 52 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6ed05f0...b93082a. Read the comment docs.

Committed-by: Diwen Zhu from Dev container
Committed-by: Diwen Zhu from Dev container
Committed-by: Diwen Zhu from Dev container

Committed-by: Diwen Zhu from Dev container
Copy link
Collaborator

@longbinlai longbinlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andydiwenzhu
Copy link
Collaborator Author

The doc is still valid, because we keep methods like graphscope.gremlin and gives hint that they will be deprecated later.

@andydiwenzhu andydiwenzhu changed the title [WIP] Support cypher query in python Support cypher query in python Jul 6, 2023
@longbinlai longbinlai merged commit 209ff2e into alibaba:main Jul 6, 2023
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[GIE] Provide Cypher instances in GraphScope's Python SDK for GIE
5 participants