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 bytecode-based requests for interactive engine #36

Merged
merged 5 commits into from
Dec 24, 2020

Conversation

siyuan0322
Copy link
Collaborator

What do these changes do?

Support bytecode-based requests for interactive engine.
User can get a GraphTraversalSource to traversal a remote graph by using interactive.traversal_source() where interactive is created by sess.gremlin(graph).

Examples:

>>> sess = graphscope.session()
>>> graph = load_modern_graph(sess, modern_graph_data_dir)
>>> interactive = sess.gremlin(graph)
>>> g = interactive.traversal_source()
>>> g.V().both()[1:3].toList()
[v[0.1], v[0.2]]
>>> g.V().both().name.toList()
['vadas', 'peter', 'lop', 'josh', 'marko', 'ripple']

Related issue number

Fixes #35

@siyuan0322
Copy link
Collaborator Author

/CC @yecol for a review,
and /CC @zhengpingq since it's an interactive engine related PR.

@codecov-io
Copy link

codecov-io commented Dec 23, 2020

Codecov Report

Merging #36 (81c8124) into main (7725f76) will increase coverage by 0.09%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #36      +/-   ##
==========================================
+ Coverage   77.26%   77.35%   +0.09%     
==========================================
  Files          48       48              
  Lines        4336     4355      +19     
==========================================
+ Hits         3350     3369      +19     
  Misses        986      986              
Impacted Files Coverage Δ
python/graphscope/deploy/tests/test_demo_script.py 99.00% <100.00%> (+0.17%) ⬆️
python/graphscope/interactive/query.py 94.64% <100.00%> (+0.41%) ⬆️

Continue to review full report at Codecov.

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

Copy link
Collaborator

@sighingnow sighingnow left a comment

Choose a reason for hiding this comment

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


It also has a method called `subgraph` which can extract some fragments
from origin graph, produce a new, smaller but concise graph stored in vineyard,
which lifetime is independent from the origin graph.

Use `execute()` to submit a script,
ans use `traversal_source()` to get a `GraphTraversalSource` for further traversal.
Copy link
Collaborator

Choose a reason for hiding this comment

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

ans -> and.

Could you please use a more readable line-breaks in comment? The following would be better:

Use `execute()` to submit a script, and use `traversal_source()` to get a `GraphTraversalSource`
for further traversal.

That doesn't affect the generated docs but will benifit the readability of source code. It would be great to think you are writing a document or paper when writing docstring in comments.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I already noticed the typo and have a fix locally, and I'm thinking combine the typo fixes with other comments to avoid another unnecessary CI.


Examples:

..code:: python
Copy link
Collaborator

Choose a reason for hiding this comment

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

Bad indent.

Examples:

.. code:: python

    xxxxx

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think sections are supposed to indent after the section header?
ref: https://www.sphinx-doc.org/en/master/usage/extensions/example_google.html#example-google

Copy link
Collaborator

Choose a reason for hiding this comment

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

I didn't realize the Examples: is a directive here, but please double check the generated doc.

Returns:
`GraphTraversalSource`
"""
return traversal().withRemote(DriverRemoteConnection(self._graph_url, "g"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the parameter "g" a hard-coded constant value?

Copy link
Collaborator Author

@siyuan0322 siyuan0322 Dec 23, 2020

Choose a reason for hiding this comment

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

Not exactly, but suffice for out scenario, just like the Client case before.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then could "g" will be used repeatly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes if you are talking about using g repeatedly across graph instances.

Copy link
Collaborator

Choose a reason for hiding this comment

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

What will happen on the same graph instance?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nothing happens, you can get two traversal source that works normally.
They are same if you look at the initialization of Client in

self._client = Client(self._graph_url, "g")

@sighingnow sighingnow self-requested a review December 23, 2020 15:16
@sighingnow sighingnow merged commit e21d946 into alibaba:main Dec 24, 2020
@siyuan0322 siyuan0322 deleted the zsy/byte branch March 3, 2021 03:40
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.

Support bytecode-based requests in interactive engine.
3 participants