harrisonhjones commented on a change in pull request #1569:
URL: https://github.com/apache/tinkerpop/pull/1569#discussion_r813058702
##########
File path: gremlin-go/README.md
##########
@@ -91,65 +121,190 @@ Note: The exact import name as well as the module prefix
for `NewDriverRemoteCon
data-flow language that enables users to succinctly express complex traversals
on (or queries of) their application's
property graph.
-Gremlin-Go implements Gremlin within the Go language and can be used on any Go
runtime greater than v1.17. Go's syntax
-has the same constructs as Java including "dot notation" for function chaining
(a.b.c), round bracket function arguments
-(a(b,c)), and support for global namespaces (a(b()) vs a(__.b())). One
important distinction with Go and Java is that
-the functions are capitalized, as is required to export functions is Go. As
such, anyone familiar with Gremlin-Java
-will immediately be able to work with Gremlin-Go.
+Gremlin-Go implements Gremlin within the Go language and can be used on any Go
runtime greater than v1.17. One important distinction with Go and Java is that
+the functions are capitalized, as is required to export functions is Go.
Review comment:
nitpick: it looks like the existing readme's text was wrapped at ~120
characters. Might want to do that here as well?
##########
File path: gremlin-go/driver/client.go
##########
@@ -41,7 +41,7 @@ type Client struct {
}
// NewClient creates a Client and configures it with the given parameters.
Review comment:
Probably also want to call out that it also creates the connection (or
attempts to) and will re-turn a non-nil error if creating this connection fails.
##########
File path: gremlin-go/README.md
##########
@@ -91,65 +121,190 @@ Note: The exact import name as well as the module prefix
for `NewDriverRemoteCon
data-flow language that enables users to succinctly express complex traversals
on (or queries of) their application's
property graph.
-Gremlin-Go implements Gremlin within the Go language and can be used on any Go
runtime greater than v1.17. Go's syntax
-has the same constructs as Java including "dot notation" for function chaining
(a.b.c), round bracket function arguments
-(a(b,c)), and support for global namespaces (a(b()) vs a(__.b())). One
important distinction with Go and Java is that
-the functions are capitalized, as is required to export functions is Go. As
such, anyone familiar with Gremlin-Java
-will immediately be able to work with Gremlin-Go.
+Gremlin-Go implements Gremlin within the Go language and can be used on any Go
runtime greater than v1.17. One important distinction with Go and Java is that
+the functions are capitalized, as is required to export functions is Go.
Gremlin-Go is designed to connect to a "server" that is hosting a
TinkerPop-enabled graph system. That "server"
could be [Gremlin Server][gs] or a [remote Gremlin provider][rgp] that exposes
protocols by which Gremlin-Go
can connect.
A typical connection to a server running on "localhost" that supports the
Gremlin Server protocol using websockets
looks like this:
-<!--
-TODO: Add Go code example of connection to server.
--->
+```go
+package main
+
+import (
+ "fmt"
+ "github.com/lyndonb-bq/tinkerpop/gremlin-go/driver"
+)
+func main() {
+ // Creating the connection to the server with default settings.
+ driverRemoteConnection, err :=
gremlingo.NewDriverRemoteConnection("localhost", 8182)
+ // Handle error
+ if err != nil {
+ fmt.Println(err)
+ return
+ }
+ // Create an anonymous traversal source with remote
+ g := gremlingo.Traversal_().WithRemote(driverRemoteConnection)
+}
+```
+We can also customize the remote connection settings. (See code documentation
for additional parameters and their usage).
+```go
+package main
+
+import (
+ "fmt"
+ "github.com/lyndonb-bq/tinkerpop/gremlin-go/driver"
+)
+
+func main() {
+ // Creating the connection to the server, changing the log verbosity to
Debug.
+ driverRemoteConnection, err :=
gremlingo.NewDriverRemoteConnection("localhost", 8182, func(settings
*gremlingo.DriverRemoteConnectionSettings) {
+ settings.LogVerbosity = gremlingo.Debug
+ })
+ // Handle error
+ if err != nil {
+ fmt.Println(err)
+ return
+ }
+ // Create an anonymous traversal source with remote
+ g := gremlingo.Traversal_().WithRemote(driverRemoteConnection)
+ // Use g with traversal as detailed in the next section.
+}
+```
Once "g" has been created using a connection, it is then possible to start
writing Gremlin traversals to query the
remote graph:
-<!--
-TODO: Add Go code example of a Gremlin traversal query.
--->
+```go
+package main
+
+import (
+ "fmt"
+ "github.com/lyndonb-bq/tinkerpop/gremlin-go/driver"
+)
-# The following material is currently Work-in-progress:
+func main() {
+ // Creating the connection to the server.
+ driverRemoteConnection, err :=
gremlingo.NewDriverRemoteConnection("localhost", 8182)
+ // Handle error
+ if err != nil {
+ fmt.Println(err)
+ return
+ }
+ // Create an anonymous traversal source with remote
+ g := gremlingo.Traversal_().WithRemote(driverRemoteConnection)
+
+ // Add a vertex with properties to the graph with the terminal step
Iterate()
+ _, promise, err := g.AddV("gremlin").Property("language",
"go").Iterate()
+ if err != nil {
+ fmt.Println(err)
+ return
+ }
+ // The returned promised is a go channel, to wait for all submitted
steps to finish execution.
+ <-promise
+
+ // Get the value of the property
+ result, err := g.V().HasLabel("gremlin").Values("language").ToList()
+ if err != nil {
+ fmt.Println(err)
+ return
+ }
+ // Print the result
+ for _, r := range result {
+ fmt.Println(r.GetString())
+ }
+}
+```
## Sample Traversals
<!--
TODO: Add Go specific changes to following paragraph:
-examples:
-"For the most part, these examples should generally translate to Go with
[little modification][differences]"
-"Given the strong correspondence between canonical Gremlin in Java and its
variants like Go, there is a limited amount
-of Go-specific documentation and examples."
-->
The Gremlin language allows users to write highly expressive graph traversals
and has a broad list of functions that
-cover a wide body of features. The [Reference Documentation][steps] describes
these functions and other aspects of the
+cover a wide body of features. Traversal in `Go` is very similar to other GLV,
with the exception that all step functions are capitalized.
+
+<!--
+The [Reference Documentation][steps] describes these functions and other
aspects of the
TinkerPop ecosystem including some specifics on [Gremlin in Go][docs] itself.
Most of the examples found in the
documentation use Groovy language syntax in the [Gremlin Console][console].
For the most part, these examples
should generally translate to Go with [little modification][differences].
Given the strong correspondence
between canonical Gremlin in Java and its variants like Go, there is a limited
amount of Go-specific
documentation and examples. This strong correspondence among variants ensures
that the general Gremlin reference
documentation is applicable to all variants and that users moving between
development languages can easily adopt the
Gremlin variant for that language.
-
-### Create Vertex
-<!--
-TODO: Add Go code to create a vertex.
-->
+### Create Vertex
+Adding a vertex with properties.
+```go
+_, promise, err :=g.AddV("gremlin").Property("language", "java").Iterate()
Review comment:
1. nitpick: missing space between `:=` and `g.AddV`
2. Should the language here still be `java`?
##########
File path: gremlin-go/README.md
##########
@@ -91,65 +121,190 @@ Note: The exact import name as well as the module prefix
for `NewDriverRemoteCon
data-flow language that enables users to succinctly express complex traversals
on (or queries of) their application's
property graph.
-Gremlin-Go implements Gremlin within the Go language and can be used on any Go
runtime greater than v1.17. Go's syntax
-has the same constructs as Java including "dot notation" for function chaining
(a.b.c), round bracket function arguments
-(a(b,c)), and support for global namespaces (a(b()) vs a(__.b())). One
important distinction with Go and Java is that
-the functions are capitalized, as is required to export functions is Go. As
such, anyone familiar with Gremlin-Java
-will immediately be able to work with Gremlin-Go.
+Gremlin-Go implements Gremlin within the Go language and can be used on any Go
runtime greater than v1.17. One important distinction with Go and Java is that
+the functions are capitalized, as is required to export functions is Go.
Gremlin-Go is designed to connect to a "server" that is hosting a
TinkerPop-enabled graph system. That "server"
could be [Gremlin Server][gs] or a [remote Gremlin provider][rgp] that exposes
protocols by which Gremlin-Go
can connect.
A typical connection to a server running on "localhost" that supports the
Gremlin Server protocol using websockets
looks like this:
-<!--
-TODO: Add Go code example of connection to server.
--->
+```go
+package main
+
+import (
+ "fmt"
+ "github.com/lyndonb-bq/tinkerpop/gremlin-go/driver"
+)
+func main() {
+ // Creating the connection to the server with default settings.
+ driverRemoteConnection, err :=
gremlingo.NewDriverRemoteConnection("localhost", 8182)
+ // Handle error
+ if err != nil {
+ fmt.Println(err)
+ return
+ }
+ // Create an anonymous traversal source with remote
+ g := gremlingo.Traversal_().WithRemote(driverRemoteConnection)
+}
+```
+We can also customize the remote connection settings. (See code documentation
for additional parameters and their usage).
+```go
+package main
+
+import (
+ "fmt"
+ "github.com/lyndonb-bq/tinkerpop/gremlin-go/driver"
+)
+
+func main() {
+ // Creating the connection to the server, changing the log verbosity to
Debug.
+ driverRemoteConnection, err :=
gremlingo.NewDriverRemoteConnection("localhost", 8182, func(settings
*gremlingo.DriverRemoteConnectionSettings) {
+ settings.LogVerbosity = gremlingo.Debug
+ })
+ // Handle error
+ if err != nil {
+ fmt.Println(err)
+ return
+ }
+ // Create an anonymous traversal source with remote
+ g := gremlingo.Traversal_().WithRemote(driverRemoteConnection)
+ // Use g with traversal as detailed in the next section.
+}
+```
Once "g" has been created using a connection, it is then possible to start
writing Gremlin traversals to query the
remote graph:
-<!--
-TODO: Add Go code example of a Gremlin traversal query.
--->
+```go
+package main
+
+import (
+ "fmt"
+ "github.com/lyndonb-bq/tinkerpop/gremlin-go/driver"
+)
-# The following material is currently Work-in-progress:
+func main() {
+ // Creating the connection to the server.
+ driverRemoteConnection, err :=
gremlingo.NewDriverRemoteConnection("localhost", 8182)
+ // Handle error
+ if err != nil {
+ fmt.Println(err)
+ return
+ }
+ // Create an anonymous traversal source with remote
+ g := gremlingo.Traversal_().WithRemote(driverRemoteConnection)
+
+ // Add a vertex with properties to the graph with the terminal step
Iterate()
+ _, promise, err := g.AddV("gremlin").Property("language",
"go").Iterate()
+ if err != nil {
+ fmt.Println(err)
+ return
+ }
+ // The returned promised is a go channel, to wait for all submitted
steps to finish execution.
+ <-promise
+
+ // Get the value of the property
+ result, err := g.V().HasLabel("gremlin").Values("language").ToList()
+ if err != nil {
+ fmt.Println(err)
+ return
+ }
+ // Print the result
+ for _, r := range result {
+ fmt.Println(r.GetString())
+ }
+}
+```
## Sample Traversals
<!--
TODO: Add Go specific changes to following paragraph:
-examples:
-"For the most part, these examples should generally translate to Go with
[little modification][differences]"
-"Given the strong correspondence between canonical Gremlin in Java and its
variants like Go, there is a limited amount
-of Go-specific documentation and examples."
-->
The Gremlin language allows users to write highly expressive graph traversals
and has a broad list of functions that
-cover a wide body of features. The [Reference Documentation][steps] describes
these functions and other aspects of the
+cover a wide body of features. Traversal in `Go` is very similar to other GLV,
with the exception that all step functions are capitalized.
+
+<!--
+The [Reference Documentation][steps] describes these functions and other
aspects of the
TinkerPop ecosystem including some specifics on [Gremlin in Go][docs] itself.
Most of the examples found in the
documentation use Groovy language syntax in the [Gremlin Console][console].
For the most part, these examples
should generally translate to Go with [little modification][differences].
Given the strong correspondence
between canonical Gremlin in Java and its variants like Go, there is a limited
amount of Go-specific
documentation and examples. This strong correspondence among variants ensures
that the general Gremlin reference
documentation is applicable to all variants and that users moving between
development languages can easily adopt the
Gremlin variant for that language.
-
-### Create Vertex
-<!--
-TODO: Add Go code to create a vertex.
-->
+### Create Vertex
+Adding a vertex with properties.
+```go
+_, promise, err :=g.AddV("gremlin").Property("language", "java").Iterate()
+// Handle error
+if err != nil {
+ fmt.Println(err)
+ return
Review comment:
nitpick: mixing tabs (Go) and spaces (not Go) here
##########
File path: gremlin-go/driver/client.go
##########
@@ -41,7 +41,7 @@ type Client struct {
}
// NewClient creates a Client and configures it with the given parameters.
-func NewClient(host string, port int, configurations ...func(settings
*ClientSettings)) *Client {
+func NewClient(host string, port int, configurations ...func(settings
*ClientSettings)) (*Client, error) {
Review comment:
This _looks_ like the functional options pattern but all the settings
are exported which isn't usually the case with that pattern. Do the settings
need to be exported?
##########
File path: gremlin-go/driver/connection.go
##########
@@ -19,50 +19,64 @@ under the License.
package gremlingo
+import (
+ "errors"
+)
+
+type connectionState int
+
+const (
+ initialized connectionState = iota + 1
+ established
+ closed
+ closedDueToError
+)
+
type connection struct {
- host string
- port int
- transporterType TransporterType
- logHandler *logHandler
- transporter transporter
- protocol protocol
- results map[string]ResultSet
+ logHandler *logHandler
+ protocol protocol
+ results map[string]ResultSet
+ state connectionState
}
-func (connection *connection) close() (err error) {
- if connection.transporter != nil {
- err = connection.transporter.Close()
- }
- return
+func (connection *connection) errorCallback() {
+ connection.logHandler.log(Error, errorCallback)
+ connection.state = closedDueToError
+ _ = connection.protocol.close()
}
-func (connection *connection) connect() error {
- if connection.transporter != nil {
- closeErr := connection.transporter.Close()
- connection.logHandler.logf(Warning, transportCloseFailed,
closeErr)
+func (connection *connection) close() error {
+ if connection.state != established {
+ return errors.New("cannot close connection that has already
been closed or has not been connected")
}
- connection.protocol = newGremlinServerWSProtocol(connection.logHandler)
- connection.transporter = getTransportLayer(connection.transporterType,
connection.host, connection.port)
- err := connection.transporter.Connect()
- if err != nil {
- return err
+ connection.logHandler.log(Info, closeConnection)
+ var err error
+ if connection.protocol != nil {
+ err = connection.protocol.close()
}
- connection.protocol.connectionMade(connection.transporter)
- return nil
+ connection.state = closed
+ return err
}
func (connection *connection) write(request *request) (ResultSet, error) {
- if connection.transporter == nil || connection.transporter.IsClosed() {
- err := connection.connect()
- if err != nil {
- return nil, err
- }
- }
- if connection.results == nil {
- connection.results = map[string]ResultSet{}
- }
+ connection.logHandler.log(Info, writeRequest)
Review comment:
thoughts on a helper `func (conn *connection) logf(...)` that simply
called `connection.logHandler.log`? Would make things a bit less verbose.
##########
File path: gremlin-go/driver/connection.go
##########
@@ -19,50 +19,64 @@ under the License.
package gremlingo
+import (
+ "errors"
+)
+
+type connectionState int
+
+const (
+ initialized connectionState = iota + 1
+ established
+ closed
+ closedDueToError
+)
+
type connection struct {
- host string
- port int
- transporterType TransporterType
- logHandler *logHandler
- transporter transporter
- protocol protocol
- results map[string]ResultSet
+ logHandler *logHandler
+ protocol protocol
+ results map[string]ResultSet
+ state connectionState
}
-func (connection *connection) close() (err error) {
- if connection.transporter != nil {
- err = connection.transporter.Close()
- }
- return
+func (connection *connection) errorCallback() {
+ connection.logHandler.log(Error, errorCallback)
+ connection.state = closedDueToError
+ _ = connection.protocol.close()
Review comment:
Any reason we are swallowing this error?
##########
File path: gremlin-go/driver/client.go
##########
@@ -71,17 +75,14 @@ func (client *Client) Close() error {
// Submit submits a Gremlin script to the server and returns a ResultSet
func (client *Client) Submit(traversalString string) (ResultSet, error) {
Review comment:
is a "traversalString" the same as a "Gremlin" script?
##########
File path: gremlin-go/driver/connection.go
##########
@@ -19,50 +19,64 @@ under the License.
package gremlingo
+import (
+ "errors"
+)
+
+type connectionState int
+
+const (
+ initialized connectionState = iota + 1
+ established
+ closed
+ closedDueToError
+)
+
type connection struct {
- host string
- port int
- transporterType TransporterType
- logHandler *logHandler
- transporter transporter
- protocol protocol
- results map[string]ResultSet
+ logHandler *logHandler
+ protocol protocol
+ results map[string]ResultSet
+ state connectionState
}
-func (connection *connection) close() (err error) {
- if connection.transporter != nil {
- err = connection.transporter.Close()
- }
- return
+func (connection *connection) errorCallback() {
+ connection.logHandler.log(Error, errorCallback)
+ connection.state = closedDueToError
+ _ = connection.protocol.close()
}
-func (connection *connection) connect() error {
- if connection.transporter != nil {
- closeErr := connection.transporter.Close()
- connection.logHandler.logf(Warning, transportCloseFailed,
closeErr)
+func (connection *connection) close() error {
+ if connection.state != established {
+ return errors.New("cannot close connection that has already
been closed or has not been connected")
}
- connection.protocol = newGremlinServerWSProtocol(connection.logHandler)
- connection.transporter = getTransportLayer(connection.transporterType,
connection.host, connection.port)
- err := connection.transporter.Connect()
- if err != nil {
- return err
+ connection.logHandler.log(Info, closeConnection)
+ var err error
+ if connection.protocol != nil {
+ err = connection.protocol.close()
}
- connection.protocol.connectionMade(connection.transporter)
- return nil
+ connection.state = closed
+ return err
}
func (connection *connection) write(request *request) (ResultSet, error) {
- if connection.transporter == nil || connection.transporter.IsClosed() {
- err := connection.connect()
- if err != nil {
- return nil, err
- }
- }
- if connection.results == nil {
- connection.results = map[string]ResultSet{}
- }
+ connection.logHandler.log(Info, writeRequest)
+ requestID := request.requestID.String()
+ connection.logHandler.logf(Info, creatingRequest, requestID)
+ connection.results[requestID] = newChannelResultSet(requestID)
+ return connection.results[requestID], connection.protocol.write(request)
+}
- // Write through protocol layer.
- responseID, err := connection.protocol.write(request,
connection.results)
- return connection.results[responseID], err
+func createConnection(host string, port int, logHandler *logHandler)
(*connection, error) {
+ conn := &connection{logHandler, nil, map[string]ResultSet{},
initialized}
+ logHandler.log(Info, connectConnection)
+ protocol, err := newGremlinServerWSProtocol(logHandler, Gorilla, host,
port, conn.results, conn.errorCallback)
+ conn.protocol = protocol
+ if err == nil {
+ conn.state = established
+ return conn, err
+ } else {
Review comment:
1. you don't need this else since you have the early return
2. usually this would be written as:
```go
if err != nil {
logHandler.logf(Error, failedConnection)
conn.state = closedDueToError
return nil, err
}
conn.state = established
return conn, err
```
3. In the `error` case does `protocol` need to be set?
##########
File path: gremlin-go/driver/serializer.go
##########
@@ -39,23 +41,47 @@ type serializer interface {
// graphBinarySerializer serializes/deserializes message to/from GraphBinary
type graphBinarySerializer struct {
- readerClass *graphBinaryReader
- writerClass *graphBinaryWriter
- mimeType string `default:"application/vnd.graphbinary-v1.0"`
+ ser *graphBinaryTypeSerializer
}
func newGraphBinarySerializer(handler *logHandler) serializer {
- reader := graphBinaryReader{handler}
- writer := graphBinaryWriter{handler}
- return graphBinarySerializer{&reader, &writer, graphBinaryMimeType}
+ serializer := graphBinaryTypeSerializer{NullType, nil, nil, nil,
handler}
+ return graphBinarySerializer{&serializer}
}
const versionByte byte = 0x81
+func convertArgs(request *request, gs graphBinarySerializer)
(map[string]interface{}, error) {
+ // TODO AN-981: Remote transaction session processor is same as bytecode
+ if request.processor == bytecodeProcessor {
+ // Convert to format:
+ // args["gremlin"]: <serialized args["gremlin"]>
+ gremlin := request.args["gremlin"]
+ switch gremlin.(type) {
+ case bytecode:
+ buffer := bytes.Buffer{}
+ gremlinBuffer, err := gs.ser.write(gremlin, &buffer)
+ if err != nil {
+ return nil, err
+ }
+ request.args["gremlin"] = gremlinBuffer
+ return request.args, nil
+ default:
+ return nil, errors.New(fmt.Sprintf("Failed to find
serializer for type '%s'.", reflect.TypeOf(gremlin).Name()))
+ }
+ } else {
+ // Use standard processor, which effectively does nothing.
+ return request.args, nil
+ }
Review comment:
it looks like everycase in the above `if` has an early return so this
else shouldn't be required.
Also, if `bytecodeProcessor` is the "normal" path consider re-writing this
as:
```go
if request.processor != bytecodeProcess {
```
##########
File path: gremlin-go/driver/connection_test.go
##########
@@ -1,50 +1,208 @@
+/*
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements. See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership. The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License. You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied. See the License for the
+specific language governing permissions and limitations
+under the License.
+*/
+
package gremlingo
import (
+ "sort"
"testing"
"github.com/stretchr/testify/assert"
"golang.org/x/text/language"
)
-const runIntegration = false
+const runIntegration = true
+const testHost string = "localhost"
+const testPort int = 8182
+const personLabel = "Person"
+const testLabel = "Test"
+const nameKey = "name"
+
+func dropGraph(t *testing.T, g *GraphTraversalSource) {
+ // Drop vertices that were added.
+ _, promise, err := g.V().Drop().Iterate()
+ assert.Nil(t, err)
+ assert.NotNil(t, promise)
+ <-promise
+}
+
+func getTestNames() []string {
+ return []string{"Lyndon", "Yang", "Simon", "Rithin", "Alexey"}
+}
Review comment:
dumb question: any reason not to just define these as a package variable?
ie:
```
package gremlingo
var testNames = []string{"Lyndon", "Yang", "Simon", "Rithin", "Alexey"}
```
##########
File path: gremlin-go/driver/serializer.go
##########
@@ -39,23 +41,47 @@ type serializer interface {
// graphBinarySerializer serializes/deserializes message to/from GraphBinary
type graphBinarySerializer struct {
- readerClass *graphBinaryReader
- writerClass *graphBinaryWriter
- mimeType string `default:"application/vnd.graphbinary-v1.0"`
+ ser *graphBinaryTypeSerializer
}
func newGraphBinarySerializer(handler *logHandler) serializer {
- reader := graphBinaryReader{handler}
- writer := graphBinaryWriter{handler}
- return graphBinarySerializer{&reader, &writer, graphBinaryMimeType}
+ serializer := graphBinaryTypeSerializer{NullType, nil, nil, nil,
handler}
+ return graphBinarySerializer{&serializer}
}
const versionByte byte = 0x81
+func convertArgs(request *request, gs graphBinarySerializer)
(map[string]interface{}, error) {
+ // TODO AN-981: Remote transaction session processor is same as bytecode
+ if request.processor == bytecodeProcessor {
+ // Convert to format:
+ // args["gremlin"]: <serialized args["gremlin"]>
+ gremlin := request.args["gremlin"]
+ switch gremlin.(type) {
+ case bytecode:
+ buffer := bytes.Buffer{}
+ gremlinBuffer, err := gs.ser.write(gremlin, &buffer)
+ if err != nil {
+ return nil, err
+ }
+ request.args["gremlin"] = gremlinBuffer
+ return request.args, nil
+ default:
+ return nil, errors.New(fmt.Sprintf("Failed to find
serializer for type '%s'.", reflect.TypeOf(gremlin).Name()))
Review comment:
1. nitpick: normally it's frowned upon to capitalize an error message.
2. you can use `%q` instead of `'%s'` unless you _want_ `'` instead of `"`
##########
File path: gremlin-go/driver/bytecode.go
##########
@@ -123,14 +126,14 @@ func (bytecode *bytecode) convertArgument(arg
interface{}) (interface{}, error)
key: v.key,
value: convertedValue,
}, nil
- // TODO: AN-970 Traversal
- //case Traversal:
- //if arg.graph != nil {
- // return errors.New()
- //}
- //for k, v := range arg.bytecode.bindings {
- // bytecode.bindings[k] = v
- //}
+ case Traversal:
+ if v.graph != nil {
+ return nil, errors.New("the child traversal was
not spawned anonymously - use the __ class rather than a TraversalSource to
construct the child traversal")
Review comment:
what is `__`?
##########
File path: gremlin-go/driver/connection.go
##########
@@ -19,50 +19,64 @@ under the License.
package gremlingo
+import (
+ "errors"
+)
+
+type connectionState int
+
+const (
+ initialized connectionState = iota + 1
+ established
+ closed
+ closedDueToError
+)
+
type connection struct {
- host string
- port int
- transporterType TransporterType
- logHandler *logHandler
- transporter transporter
- protocol protocol
- results map[string]ResultSet
+ logHandler *logHandler
+ protocol protocol
+ results map[string]ResultSet
+ state connectionState
}
-func (connection *connection) close() (err error) {
- if connection.transporter != nil {
- err = connection.transporter.Close()
- }
- return
+func (connection *connection) errorCallback() {
+ connection.logHandler.log(Error, errorCallback)
+ connection.state = closedDueToError
+ _ = connection.protocol.close()
}
-func (connection *connection) connect() error {
- if connection.transporter != nil {
- closeErr := connection.transporter.Close()
- connection.logHandler.logf(Warning, transportCloseFailed,
closeErr)
+func (connection *connection) close() error {
+ if connection.state != established {
+ return errors.New("cannot close connection that has already
been closed or has not been connected")
}
- connection.protocol = newGremlinServerWSProtocol(connection.logHandler)
- connection.transporter = getTransportLayer(connection.transporterType,
connection.host, connection.port)
- err := connection.transporter.Connect()
- if err != nil {
- return err
+ connection.logHandler.log(Info, closeConnection)
+ var err error
+ if connection.protocol != nil {
+ err = connection.protocol.close()
}
- connection.protocol.connectionMade(connection.transporter)
- return nil
+ connection.state = closed
Review comment:
here and everywhere else `connection` is accessed/mutated: does
`connection` need to be safe to access concurrently? If so, we might want to
introduce a mutex for locking
##########
File path: gremlin-go/driver/logger.go
##########
@@ -32,7 +34,7 @@ type LogVerbosity int
const (
// Debug verbosity will log everything, including fine details.
- Debug LogVerbosity = iota
+ Debug LogVerbosity = iota + 1
Review comment:
Why the plus one? Trying to avoid the 0 zero value?
##########
File path: gremlin-go/driver/result.go
##########
@@ -21,18 +21,206 @@ package gremlingo
// AN-968 Finish Result implementation.
-import "fmt"
+import (
+ "errors"
+ "fmt"
+ "reflect"
+ "strconv"
+)
// Result Struct to abstract the Result and provide functions to use it.
type Result struct {
result interface{}
}
-// AsString returns the current Result formatted as a string.
-func (r *Result) AsString() string {
+// ToString returns the string representation of the Result struct in
Go-syntax format
+func (r *Result) ToString() string {
Review comment:
`ToString` is kind of a JavaScript-ism to me. `String` would be more
idiomatic imo
##########
File path: gremlin-go/driver/client.go
##########
@@ -71,17 +75,14 @@ func (client *Client) Close() error {
// Submit submits a Gremlin script to the server and returns a ResultSet
func (client *Client) Submit(traversalString string) (ResultSet, error) {
// TODO AN-982: Obtain connection from pool of connections held by the
client.
+ client.logHandler.logf(Debug, submitStartedString, traversalString)
request := makeStringRequest(traversalString)
- if client.connection == nil {
- client.connection = &connection{
- host: client.host,
- port: client.port,
- transporterType: client.transporterType,
- logHandler: client.logHandler,
- transporter: nil,
- protocol: nil,
- }
- }
+ return client.connection.write(&request)
+}
+// SubmitBytecode submits bytecode to the server to execute and returns a
ResultSet
Review comment:
nitpick: ideally doc comments (according to https://go.dev/blog/godoc)
should be sentences ended with proper punctuation.
##########
File path: gremlin-go/README.md
##########
@@ -91,65 +121,190 @@ Note: The exact import name as well as the module prefix
for `NewDriverRemoteCon
data-flow language that enables users to succinctly express complex traversals
on (or queries of) their application's
property graph.
-Gremlin-Go implements Gremlin within the Go language and can be used on any Go
runtime greater than v1.17. Go's syntax
-has the same constructs as Java including "dot notation" for function chaining
(a.b.c), round bracket function arguments
-(a(b,c)), and support for global namespaces (a(b()) vs a(__.b())). One
important distinction with Go and Java is that
-the functions are capitalized, as is required to export functions is Go. As
such, anyone familiar with Gremlin-Java
-will immediately be able to work with Gremlin-Go.
+Gremlin-Go implements Gremlin within the Go language and can be used on any Go
runtime greater than v1.17. One important distinction with Go and Java is that
+the functions are capitalized, as is required to export functions is Go.
Gremlin-Go is designed to connect to a "server" that is hosting a
TinkerPop-enabled graph system. That "server"
could be [Gremlin Server][gs] or a [remote Gremlin provider][rgp] that exposes
protocols by which Gremlin-Go
can connect.
A typical connection to a server running on "localhost" that supports the
Gremlin Server protocol using websockets
looks like this:
-<!--
-TODO: Add Go code example of connection to server.
--->
+```go
+package main
+
+import (
+ "fmt"
+ "github.com/lyndonb-bq/tinkerpop/gremlin-go/driver"
+)
+func main() {
+ // Creating the connection to the server with default settings.
+ driverRemoteConnection, err :=
gremlingo.NewDriverRemoteConnection("localhost", 8182)
+ // Handle error
+ if err != nil {
+ fmt.Println(err)
+ return
+ }
+ // Create an anonymous traversal source with remote
+ g := gremlingo.Traversal_().WithRemote(driverRemoteConnection)
+}
+```
+We can also customize the remote connection settings. (See code documentation
for additional parameters and their usage).
+```go
+package main
+
+import (
+ "fmt"
+ "github.com/lyndonb-bq/tinkerpop/gremlin-go/driver"
+)
+
+func main() {
+ // Creating the connection to the server, changing the log verbosity to
Debug.
+ driverRemoteConnection, err :=
gremlingo.NewDriverRemoteConnection("localhost", 8182, func(settings
*gremlingo.DriverRemoteConnectionSettings) {
+ settings.LogVerbosity = gremlingo.Debug
+ })
+ // Handle error
+ if err != nil {
+ fmt.Println(err)
+ return
+ }
+ // Create an anonymous traversal source with remote
+ g := gremlingo.Traversal_().WithRemote(driverRemoteConnection)
+ // Use g with traversal as detailed in the next section.
+}
+```
Once "g" has been created using a connection, it is then possible to start
writing Gremlin traversals to query the
remote graph:
-<!--
-TODO: Add Go code example of a Gremlin traversal query.
--->
+```go
+package main
+
+import (
+ "fmt"
+ "github.com/lyndonb-bq/tinkerpop/gremlin-go/driver"
+)
-# The following material is currently Work-in-progress:
+func main() {
+ // Creating the connection to the server.
+ driverRemoteConnection, err :=
gremlingo.NewDriverRemoteConnection("localhost", 8182)
+ // Handle error
+ if err != nil {
+ fmt.Println(err)
+ return
+ }
+ // Create an anonymous traversal source with remote
+ g := gremlingo.Traversal_().WithRemote(driverRemoteConnection)
+
+ // Add a vertex with properties to the graph with the terminal step
Iterate()
+ _, promise, err := g.AddV("gremlin").Property("language",
"go").Iterate()
+ if err != nil {
+ fmt.Println(err)
+ return
+ }
+ // The returned promised is a go channel, to wait for all submitted
steps to finish execution.
+ <-promise
+
+ // Get the value of the property
+ result, err := g.V().HasLabel("gremlin").Values("language").ToList()
+ if err != nil {
+ fmt.Println(err)
+ return
+ }
+ // Print the result
+ for _, r := range result {
+ fmt.Println(r.GetString())
+ }
+}
+```
## Sample Traversals
<!--
TODO: Add Go specific changes to following paragraph:
-examples:
-"For the most part, these examples should generally translate to Go with
[little modification][differences]"
-"Given the strong correspondence between canonical Gremlin in Java and its
variants like Go, there is a limited amount
-of Go-specific documentation and examples."
-->
The Gremlin language allows users to write highly expressive graph traversals
and has a broad list of functions that
-cover a wide body of features. The [Reference Documentation][steps] describes
these functions and other aspects of the
+cover a wide body of features. Traversal in `Go` is very similar to other GLV,
with the exception that all step functions are capitalized.
+
+<!--
+The [Reference Documentation][steps] describes these functions and other
aspects of the
TinkerPop ecosystem including some specifics on [Gremlin in Go][docs] itself.
Most of the examples found in the
documentation use Groovy language syntax in the [Gremlin Console][console].
For the most part, these examples
should generally translate to Go with [little modification][differences].
Given the strong correspondence
between canonical Gremlin in Java and its variants like Go, there is a limited
amount of Go-specific
documentation and examples. This strong correspondence among variants ensures
that the general Gremlin reference
documentation is applicable to all variants and that users moving between
development languages can easily adopt the
Gremlin variant for that language.
-
-### Create Vertex
-<!--
-TODO: Add Go code to create a vertex.
-->
+### Create Vertex
+Adding a vertex with properties.
+```go
+_, promise, err :=g.AddV("gremlin").Property("language", "java").Iterate()
+// Handle error
+if err != nil {
+ fmt.Println(err)
+ return
+}
+// Wait for all steps to finish execution
+<-promise
+```
### Find Vertices
-<!--
-TODO: Add Go code for Find Vertices.
--->
+Getting the property value associated with the added vertex. We currently only
support `ToList()` for submitting the remote traversal. Support for `Next()`
will be implemented in the subsequent milestones.
+```go
+result, err := g.V().HasLabel("gremlin").Values("language").ToList()
+// Handle error
+if err != nil {
+fmt.Println(err)
+return
Review comment:
nitpick: missing tabs here (and a bit further down)
##########
File path: gremlin-go/driver/connection.go
##########
@@ -19,50 +19,64 @@ under the License.
package gremlingo
+import (
+ "errors"
+)
+
+type connectionState int
+
+const (
+ initialized connectionState = iota + 1
+ established
+ closed
+ closedDueToError
+)
+
type connection struct {
- host string
- port int
- transporterType TransporterType
- logHandler *logHandler
- transporter transporter
- protocol protocol
- results map[string]ResultSet
+ logHandler *logHandler
+ protocol protocol
+ results map[string]ResultSet
+ state connectionState
}
-func (connection *connection) close() (err error) {
- if connection.transporter != nil {
- err = connection.transporter.Close()
- }
- return
+func (connection *connection) errorCallback() {
+ connection.logHandler.log(Error, errorCallback)
+ connection.state = closedDueToError
+ _ = connection.protocol.close()
}
-func (connection *connection) connect() error {
- if connection.transporter != nil {
- closeErr := connection.transporter.Close()
- connection.logHandler.logf(Warning, transportCloseFailed,
closeErr)
+func (connection *connection) close() error {
+ if connection.state != established {
+ return errors.New("cannot close connection that has already
been closed or has not been connected")
}
- connection.protocol = newGremlinServerWSProtocol(connection.logHandler)
- connection.transporter = getTransportLayer(connection.transporterType,
connection.host, connection.port)
- err := connection.transporter.Connect()
- if err != nil {
- return err
+ connection.logHandler.log(Info, closeConnection)
+ var err error
+ if connection.protocol != nil {
+ err = connection.protocol.close()
}
- connection.protocol.connectionMade(connection.transporter)
- return nil
+ connection.state = closed
+ return err
}
func (connection *connection) write(request *request) (ResultSet, error) {
Review comment:
here and elsewhere. Does `connection` need to be so verbose? Thoughts on
`func (conn *connection)` instead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
