-
Notifications
You must be signed in to change notification settings - Fork 811
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
Update documentation around TinkerPop HTTP API and serializers. #2908
base: master
Are you sure you want to change the base?
Conversation
docs/src/dev/io/graphson.asciidoc
Outdated
generalized object serialization format. That characteristic makes it useful as a serialization format for Gremlin | ||
Server where arbitrary objects of varying types may be returned as results. However, starting in GraphSON 4, GraphSON | ||
is only intended to be a network serialization format that is only able to serialize specific types defined by the | ||
format. It is only meant to be used between language variants and the Gremlin Server. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is only meant to be used between language variants and the Gremlin Server.
is it just language variants? that seems to rule out pure http based requests, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it fair to say that GraphSON 4 is a useful "graph format" for any graph which is limited to containing TinkerPop serializable types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it just language variants? that seems to rule out pure http based requests, no?
updated to say that its intended to work with the HTTP API
Is it fair to say that GraphSON 4 is a useful "graph format" for any graph which is limited to containing TinkerPop serializable types?
sure, but I don't think it would ever be used that way so I wouldn't phrase it that way
docs/src/dev/provider/index.asciidoc
Outdated
@@ -54,6 +54,168 @@ This document attempts to address the needs of the different providers that have | |||
* Graph Language Provider | |||
* Graph Plugin Provider | |||
|
|||
== HTTP API |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something about the ordering of the Provider doc feels off to me. Someone implementing TinkerPop is the target reader. They likely would not start implementing by doing "HTTP API". The "Graph System Provider Requirements" are probably where most implementers would start their work. Could you give a bit of thought to organization and placement of this section?
I'd suggest a paragraph at the end of the "Provider Documentation" section above that says something like:
Deciding which of the interfaces, protocols, and tests to implement depends largely on where your work fits in the TinkerPop system and how advanced you intend for that implementation to be. The following bullet points detail common provider projects that implementers undertake and what parts of the documentation will be most relevant to them in getting started:
- Building a TinkerPop compliant graph database
- Building a Gremlin Server implementation to provide remote connectivity over HTTP and with Gremlin drivers
- Building a driver to work with Gremlin Server implementations
- Building a Gremlin execution engine
The bullets are just possible sections that could likely replace the bullets above that just list the types of providers. Each could have some sort of introductory paragraph that gives an overview of what's involved in those tasks, links to docs we have and other hints to get started. I think that would introduce the sections below much better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had it this way because I thought that the HTTP API was something both driver providers and graph providers should know, but I guess only driver providers need to directly implement it. I've pushed this whole section back into the graph driver providers section
docs/src/dev/provider/index.asciidoc
Outdated
|Key |Description |Value |Required | ||
|gremlin |The Gremlin query to execute. |String containing script |Yes | ||
|timeoutMs |The maximum time a query is allowed to execute in milliseconds. |Number between 0 and 2^31-1 |No | ||
|bindings |Any bindings used to execute the query. |Object (Map) |No |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you clarify "bindings" a bit? or link to their meaning if described elsewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated with more information on its interaction with "language" and how bindings are used based on it.
docs/src/dev/provider/index.asciidoc
Outdated
|g |The name of the graph traversal source to which the query applies. Default: "g" |String containing traversal source name |No | ||
|language |The name of the ScriptEngine to use to parse the gremlin query. Default: "gremlin-lang" |String containing ScriptEngine name |No | ||
|materializeProperties |Whether to include all properties for results. One of "tokens" or "all". |String |No | ||
|bulked |Whether the result should be "bulked" (only applies to GraphBinary) |Boolean |No |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could you clarify "bulked" a bit? or link to their meaning if described elsewhere?
Let's use the incoming request to process the Gremlin script of `g.V()` as an example. Gremlin Server evaluates that | ||
script, getting an `Iterator` of vertices as a result, and steps through each `Vertex` within it. The vertices are | ||
batched together into an HTTP chunk. Each response is serialized given the requested serializer type (GraphBinary is | ||
recommended) and written back to the requesting client immediately. Gremlin Server does not wait for the entire result |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the docs keep mentioning "GraphBinary is recommended" but do we say anywhere why this is the case? ultimately, GraphBinary will effectively be the only option for drivers, so perhaps the docs should just be more clear that this is just a M1 recommendation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added new "serializer" section to explain more in-depth why GraphBinary is recommended.
request interceptor. Refer to your provider's documentation to determine if other authentication mechanisms are | ||
available. | ||
|
||
==== Transactions Disabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm, this is true of M1, but not what will be 4.0 GA. Are these upgrade docs for the M1 or for 4.0 GA? i kinda feel like its the latter and that we need a different place to call out milestone limitations. how about adding:
== TinkerPop 4.0.0.M1
The 4.0.0.M1 is a milestone release. It is for meant as a preview version to try out the new HTTP API features in the the server and drivers. As this is a milestone version only, you can expect breaking changes to occur in future milestones for 4.0.0 on the way to its General Availability release. The following sections detail important limitations and constraints pertinent to this milestone that may or may not apply to General Availability.
as a new section and anything that is specific to M1 goes in there with
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the version at the top from 4.0.0 to 4.0.0.M1 and added a disclaimer in that section. The disclaimer says that there was an attempt to tag parts that might change with an IMPORTANT box.
for more detailed information. The subprotocol remains fairly similar but has been adjusted to work better with HTTP. | ||
Also, the move to HTTP means that SASL has been removed as an authentication mechanism and only HTTP basic remains. | ||
|
||
===== Request Interceptor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as mentioned elsewhere "recommended" is fine, but it might be help to be more explicit as to "why" by mentioning that not allowing for interceptor capabilities may preclude the driver from working with certain providers.
@@ -102,13 +104,11 @@ mime type is made explicit on requests to avoid breaking changes or unexpected r | |||
|
|||
Version 4.0 of GraphSON was first introduced on TinkerPop 4.0.0 and is represented by the | |||
`application/vnd.gremlin-v4.0+json` mime type. There also exists an untyped version: | |||
`application/vnd.gremlin-v3.0+json;types=false`. It is very similar to GraphSON 3.0, with just several key differences: | |||
`application/vnd.gremlin-v4.0+json;types=false`. It is very similar to GraphSON 3.0, with just several key differences: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`application/vnd.gremlin-v4.0+json;types=false`. It is very similar to GraphSON 3.0, with just several key differences: | |
`application/vnd.gremlin-v4.0+json;types=false`. It is very similar to GraphSON 4.0, with just several key differences: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that part is supposed to say GraphSON 3.0.
@@ -2988,8 +2980,6 @@ The following `ResponseMessage` is a typical example of the typical successful r | |||
} | |||
---- | |||
|
|||
=== Extended | |||
|
|||
Note that the "extended" types require the addition of the separate `GraphSONXModuleV4d0` module as follows: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This note needs updating if we are discarding the Core and Extended types categories
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to leave it as is. GraphSONXModule still exists so it might still be proper to call them "extended" even though those sections in the document have been removed. What do you think?
Please look over this documentation change for either inaccurate information or missing information.
VOTE +1