You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by jorgebay <gi...@git.apache.org> on 2016/10/05 14:30:27 UTC
[GitHub] tinkerpop pull request #450: Javascript GLV
GitHub user jorgebay opened a pull request:
https://github.com/apache/tinkerpop/pull/450
Javascript GLV
For [TINKERPOP-1489](https://issues.apache.org/jira/browse/TINKERPOP-1489).
- Should work with any ES5 engine that supports CommonJs: tested with Nashorn and Node.js.
- Maintained the same structure as the Python GLV.
- Used groovy classes to generate all the traversal methods.
- Javascript tests run in the test maven phase (JS test names are not printed though...)
- Javascript engines are designed to run user code in a single thread, IO libraries (like libuv on Node.js) are async only. With that in mind, there isn't sync IO methods exposed in the Traversal (ie: `#next()`, `#toList()`). Instead I exposed `#list()` and `one()` that take a callback as a parameter:
```javascript
g.V().hasLabel('software').list((err, vertices) => {
vertices.forEach(console.log);
});
g.V().has('name','marko').one((err, vertex) => {
console.log(vertex.label); // person
});
```
This patch is focused in providing a Javascript GLV that would be useful for most runtimes, most notably Node.js. It implements a Graph, GraphTraversal and GraphTraversalSource, along with GraphSONReader and GraphSONWriter.
It includes base classes for RemoteConnection, RemoteTraversal and RemoteStrategy, but doesn't include DriverRemoteConnection implementation as Javascript engines does not provide a standard IO library.
As a first step, the idea is to include in the TinkerPop project the specification for the language variant, the serialization functionality and the execution methods (currently named `list()` and `one()`).
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jorgebay/tinkerpop javascript-glv
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/450.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #450
----
commit 5f7a670bceedd0c65f5c1b8e00a06f4d8cdac912
Author: Jorge Bay Gondra <jo...@gmail.com>
Date: 2016-10-05T14:14:46Z
Javascript GLV
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #450: TINKERPOP-1489 Javascript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/450
I like the idea about returning promises. I think it could be a method overload: user provides a function, use it as callback; otherwise, return a promise. For runtimes that don't support promises, if no callback is provided, throw an error. That way we can support all ES5+ runtimes (nashorn, nodejs, browser js engines).
Example:
```javascript
// callback provided
g.V().hasLabel('person').toList((err, people) => {
people.forEach(console.log);
});
// callback not provided, a promise is returned
const peoplePromise = g.V().hasLabel('software').toList();
```
Promises in the Javascript landscape had the added benefit that can be used along side generators (ES2015) and async/await (ES2017) in newer runtimes.
```javascript
// inside a generator
const people = yield peoplePromise;
// inside an async function
const people = await peoplePromise;
```
I think that if we provide both callback and promise method overloads, we can avoid the need of a Promise polyfill.
About method generation:
I think the approach used by the Python GLV of generating methods is still more user friendly than capture a function invocation. Using proxies is an elegant solution, but compile time generated methods have several benefits over [Javascript Proxy][1] approach:
- Compilers can optimize generated code.
- We can filter out Traversal/TraversalSource methods that we want to handle differently.
- Node.js active LTS (v4) and most popular runtimes don't support it yet.
- The generated source is a defined API.
That way we could use the same approach from Python for all supported languages.
[1]: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Proxy
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #450: TINKERPOP-1489 Javascript GLV
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/450
ha +1 for a promise style api - i just wrote the exact same suggestion for the java side:
https://issues.apache.org/jira/browse/TINKERPOP-1490?focusedCommentId=15557840&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15557840
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #450: TINKERPOP-1489 Javascript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/450
I've added some fixes during the past weeks.
I'll try to summarize the open issues with this patch to try to unblock it:
A) Currently, the javascript GLV exposes `list()` instead of `toList()`. As it takes a callback as a parameter, having the method start with a `toX()` didn't feel natural to me.
Maybe it makes more sense to follow the existing naming (`toList()`). Exposing something uniform across all GLVs, whenever possible, can be more important (in the same way as Python GLV used mixedCase to be more close to gremlin).
The same applies to `next()`. To avoid confusions, we could document that `traversal.next()` is not an iterator and returns the first result.
B) Promise-based API.
We can include a promise-based method overloads when supported by the js engine, like [proposed above][1]. We can do it now, before merging it or as a next step after merging.
Can we try to reach a conclusion on these issues?
After that I can send a draft documentation for `gremlin-variants.asciidoc`.
I've been using the Javascript GLV for a while now and it feels good to have code completion for Traversal methods on the IDE!
[1]: https://github.com/apache/tinkerpop/pull/450#issuecomment-252911095
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #450: TINKERPOP-1489 Javascript GLV
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/450
@jorgebay -- thanks for being patient. We have pushed off thinking about this since it wasn't going into the 3.2.3 and we were dealing with that release the last 2+ weeks.
I would like to get this into 3.2.4, so, lets focus on it again!
We need to get the promise-based API settled for Gremlin-Java and then we can ripple that to Gremlin-JavaScript. Thus, I think we should solve and merge this ticket first: https://issues.apache.org/jira/browse/TINKERPOP-1490
I'm not too smart in this type of work ("promise" "future" etc. APIs)... so help on that ticket would be appreciated. Also, would having this worked out, then help to solidify your open questions on Gremlin-JavaScript?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #450: Javascript GLV
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/450
@jorgebay thanks for this. I have a feeling it will take a some time to review this work and get community input. hopefully @jbmusso will have time to get involved.
in the mean time could you please rename your pull request to include the JIRA number as the prefix - something like: "TINKERPOP-1489 Javascript GLV" Apache automation will then be able to do it's work with JIRA.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #450: TINKERPOP-1489 Javascript GLV
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/450#discussion_r82161973
--- Diff: gremlin-javascript/pom.xml ---
@@ -0,0 +1,132 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ 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.
+ -->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>tinkerpop</artifactId>
+ <version>3.2.3-SNAPSHOT</version>
+ </parent>
+ <artifactId>gremlin-javascript</artifactId>
+ <name>Apache TinkerPop :: Gremlin Javascript</name>
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>gremlin-core</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.codehaus.groovy</groupId>
+ <artifactId>groovy</artifactId>
+ <version>${groovy.version}</version>
+ <classifier>indy</classifier>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>tinkergraph-gremlin</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>gremlin-test</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>gremlin-server</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-log4j12</artifactId>
+ <version>${slf4j.version}</version>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+ <properties>
+ <!-- provides a way to convert maven.test.skip value to skipTests for use in skipping python tests -->
+ <maven.test.skip>false</maven.test.skip>
+ <skipTests>${maven.test.skip}</skipTests>
+ <gremlin.server.dir>${project.parent.basedir}/gremlin-server</gremlin.server.dir>
+ </properties>
+ <build>
+ <directory>${basedir}/target</directory>
+ <finalName>${project.artifactId}-${project.version}</finalName>
+ <plugins>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>exec-maven-plugin</artifactId>
+ <version>1.2.1</version>
+ <executions>
+ <execution>
+ <id>generate-javascript</id>
+ <phase>generate-test-resources</phase>
+ <goals>
+ <goal>java</goal>
+ </goals>
+ <configuration>
+ <mainClass>org.apache.tinkerpop.gremlin.javascript.GenerateGremlinJavascript</mainClass>
+ <arguments>
+ <argument>${basedir}/src/main/javascript/gremlin-javascript/process/traversal.js</argument>
+ <argument>${basedir}/src/main/javascript/gremlin-javascript/process/graph-traversal.js</argument>
+ </arguments>
+ </configuration>
+ </execution>
+ <execution>
+ <id>js-tests</id>
--- End diff --
@jorgebay can you explain the nature of this test execution? Does maven fail the build if these test fail? what kind of output does it produce?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #450: TINKERPOP-1489 Javascript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/450
ok thanks!
I hope I'll get some time soon to work on the JavaScript GLV to leverage the new async execution introduced in TINKERPOP-1490.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #450: TINKERPOP-1489 Javascript GLV
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/450
I did a preliminary review of the code and it looks good. Primarily because it mimics the structure and content of Gremlin-Python. Ensuring consistency between all variants is important from a maintainability standpoint.
Here are some concerns:
1. I think we need a `RemoteConnection` implementation. From what I'm reading, it seems we don't have one.
2. @mbroecheler was pushing for async methods in Gremlin-Java (thus, "Gremlin"). The idea was to have `Future<V> Traversal.async(Function<Traversal,V>)`. Thus, to do a "future" `toList()` in Java, you would do `result = g.V().out().async(Traversal::toList)`. I think we should do this in Gremlin-Java and then have this same "callback" model used by Gremlin-JS and thus, not have `one()` and `list()` with callbacks. Does that make sense?
3. I don't see the Gremlin `ProcessTestSuite` being run. We will need `Providers` to do so. Please see how this works in Gremlin-Python. https://github.com/apache/tinkerpop/tree/master/gremlin-python/src/test/java/org/apache/tinkerpop/gremlin/python/jsr223.
4. We will need documentation in `gremlin-variants.asciidoc`.
I suspect we will want to merge this first into an Apache TinkerPop branch and can nit pick things as I see them before an ultimate merge into `master/`. For instance, I can do 3 and 4 above if perhaps @jorgebay provides some notes so I get the important aspects in the documentation.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #450: TINKERPOP-1489 Javascript GLV
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/450
sorry - changed this up. i created TINKERPOP-1489 branch for js work and TINKERPOP-1552 for c# work. both branches extend from tp32-glv. in this way, the GLVs are not coupled together to be forced to be merged together. they can be developed at their own pace. note that tp32-glv can house changes useful to both of these GLVs (stuff like the revised testing framework).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #450: Javascript GLV
Posted by PommeVerte <gi...@git.apache.org>.
Github user PommeVerte commented on the issue:
https://github.com/apache/tinkerpop/pull/450
Nice PR @jorgebay
Can you elaborate on why you went for `list()` and `one()` rather than keeping the original method names of `toList()` and `next()`? I understand that they would still have to require callbacks but I guess the point I'm trying to make is that it would probably be better to stick to the original naming to stay consistent with gremlin documentation, and then document the variations in a JS GLV specific documentation.
So :
```javascript
g.V().hasLabel('software').toList((err, vertices) => {
vertices.forEach(console.log);
});
g.V().has('name','marko').next((err, vertex) => {
console.log(vertex.label); // person
});
```
Thoughts?
Also, you mentioned not including the Driver because Javascript engines does not provide a standard IO library. What would your suggestion be here? Was that just an invitation for the community to figure out if we should use an existing library or write a new driver?
Do you have any insight here in regards to something like [gremlin-javascript](https://github.com/jbmusso/gremlin-javascript)? (minus that it's not
ECMAScript5 native and doesn't currently support ByteCode).
Anyways, I think we would proceed to rewrite a minimal version for the GLV anyways.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #450: TINKERPOP-1489 Javascript GLV
Posted by jbmusso <gi...@git.apache.org>.
Github user jbmusso commented on the issue:
https://github.com/apache/tinkerpop/pull/450
I was thinking that supporting JavaScript `Promise` could be helpful now that they\u2019ve been part of the ES2015 and rolled out on most JS environment (browsers/Node.js). Maybe this can be polyfilled under Nashorn? Using `Promise` could complement Node.js style async functions with a callback passed as last parameter (functions with such signature can be easily converted to functions returning a Promise [with many third-party Promise libraries such as Bluebird](http://bluebirdjs.com/docs/api/promise.promisify.html)). This way, a `Traversal` could be ended with something like `.toPromise()`. I think the following makes a lot of sense in JavaScript since it\u2019s part of the standard:
```javascript
// JavaScript
g.V().out('created').toPromise()
.then(function (vertices) {
// do something with results
})
.catch(function (err) {
// handle error
})
.finally(function () {
// cleanup
});
// Alternatively, the end user could use use a promisifying function over .toList():
g.V().out('created').toListAsync()
.then()
.catch();
```
Coming soon to the standard are `async` functions which return a `Promise`, so you could do this:
```javascript
// JavaScript
async function followers() {
try {
const followers = await g.V().in('followers').toPromise();
} catch(e) {
// Handle error
}
}
```
There also are `Observable` (think: `Promise` with multiple return values, or `Array` but laid out over time rather than in memory) that are worth considering in JavaScript. Observable are candidate for being part of the standard. Libraries such as RxJS v5, which aims to be spec. compliant, works wonderfully. That way, we could also have `.toObservable()` in JavaScript:
```javascript
// JavaScript
g.V().out('created').toObservable()
.subscribe(
function (vertex) {
// This gets called once for each vertex
},
function (err) {
// Handle errors
},
function () {
// Called when observable ends (ie. after last vertex is emitted)
}
)
```
I took a different path in Node.js and started developing [zer](https://github.com/jbmusso/zer) (short for seriali**zer**), a generic "JavaScript-to-anything" library that uses the ES2015 Proxy object. The lib builds an AST-ish (I'm learning these :P) which can output to anything, including Gremlin-Groovy (currently) and most likely Gremlin-bytcode easily (it could also output to SQL or Cypher, but hey). The trick is to `Proxy()`ify a `Function` (rather than an `Object`) and intercept calls to that function with `Proxy.apply()` and mutation of that function properties with `Proxy.get()` (you can do both since functions are objects in JavaScript).
The `zer` lib allows generating queries such as `g.V().out().in().bla().method().not().in().tinkerpop().source().code()` (Grooovy output) but it could easily support things like `select().from().where()` (and produce SQL-output). Coding the gremlin-bytecode output should be trivial. It also supports nested traversals and automatic escaping of bound parameters (it basically escapes all primitives and anything that is not a `Traversal`). I think it could also support serializing functions/lambdas since you can do `Function.toString()` in JavaScript and extract the function body at runtime.
I don\u2019t think that the Proxy approach can work in Nashorn since Proxy can\u2019t be emulated in ES5 environment (it only works in Node v6 I recall, maybe Node v5). However, in Node.js, I think I would just use Proxy objects that can intercept anything - I think generating code might not be worth it there. I think this PR is valid in Nashorn but I'm unsure about Node.js.
Note that `zer` and `gremlin-javascript` are distinct libraries. Once stable enough, `zer` (with just Gremlin-Groovy and Gremlin-Bytecode output) could be added as a dependency to `gremlin-javascript`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #450: TINKERPOP-1489 Javascript GLV
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/450
I just created this ticket: https://issues.apache.org/jira/browse/TINKERPOP-1490
@jorgebay -- can you review that ticket in terms of the `one()`, `list()` callback model you are using here to see if we can get rid of `one()` and `list()` and supplant it with an `async()`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #450: TINKERPOP-1489 Javascript GLV
Posted by jbmusso <gi...@git.apache.org>.
Github user jbmusso commented on the issue:
https://github.com/apache/tinkerpop/pull/450
Ah, it looks like [nashorn supports let and const keywords on 8u40](http://fr.slideshare.net/akihironishikawa/nashorn-in-the-future-en) so we could swap `var` for `let` and `const` unless we want to target earlier versions of Java8. I don't think adding a transpiler such as Babeljs to the mix is worth the trouble here since it's going to be a nightmare to maintain the build pipeline. However, it *could* make sense for more recent versions of Node. Node 7 would be happier with ES2015 code rather than ES5 ;) - see http://node.green/. I don't think it's realistic so I'm just putting it here for further reference, basically when Java9 is out (where [nashorn could fully support ES2015/6](http://fr.slideshare.net/akihironishikawa/nashorn-in-the-future-en)).
Note that there is [a proposal for adding async iterator to JavaScript](https://github.com/tc39/proposal-async-iteration). It's currently in stage 3 so this could make it to the EcmaScript specification. Using `.next()` for async iteration might become a reality.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #450: TINKERPOP-1489 Javascript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/450
1: I mentioned the difficulty of delivering a `RemoteConnection` implementation inside the TinkerPop repository above.
2: I commented in TINKERPOP-1490 that `async` is a reserved keyword, in this case, in Javascript. We could use for the Javascript GLV the method names that comes from the discussion in that ticket.
3: I will need more time and guidance to deliver GremlinJavascriptScriptEngine. Things like `org.apache.tinkerpop.gremlin.util['function'].Lambda.ZeroArgLambda` are failing under nashorn because ZeroArgLambda is being identified as a package but not as a class (js constructor).
4: I can send some notes and code samples your way.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #450: TINKERPOP-1489 Javascript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/450#discussion_r82163673
--- Diff: gremlin-javascript/pom.xml ---
@@ -0,0 +1,132 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ 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.
+ -->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>tinkerpop</artifactId>
+ <version>3.2.3-SNAPSHOT</version>
+ </parent>
+ <artifactId>gremlin-javascript</artifactId>
+ <name>Apache TinkerPop :: Gremlin Javascript</name>
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>gremlin-core</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.codehaus.groovy</groupId>
+ <artifactId>groovy</artifactId>
+ <version>${groovy.version}</version>
+ <classifier>indy</classifier>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>tinkergraph-gremlin</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>gremlin-test</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>gremlin-server</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-log4j12</artifactId>
+ <version>${slf4j.version}</version>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+ <properties>
+ <!-- provides a way to convert maven.test.skip value to skipTests for use in skipping python tests -->
+ <maven.test.skip>false</maven.test.skip>
+ <skipTests>${maven.test.skip}</skipTests>
+ <gremlin.server.dir>${project.parent.basedir}/gremlin-server</gremlin.server.dir>
+ </properties>
+ <build>
+ <directory>${basedir}/target</directory>
+ <finalName>${project.artifactId}-${project.version}</finalName>
+ <plugins>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>exec-maven-plugin</artifactId>
+ <version>1.2.1</version>
+ <executions>
+ <execution>
+ <id>generate-javascript</id>
+ <phase>generate-test-resources</phase>
+ <goals>
+ <goal>java</goal>
+ </goals>
+ <configuration>
+ <mainClass>org.apache.tinkerpop.gremlin.javascript.GenerateGremlinJavascript</mainClass>
+ <arguments>
+ <argument>${basedir}/src/main/javascript/gremlin-javascript/process/traversal.js</argument>
+ <argument>${basedir}/src/main/javascript/gremlin-javascript/process/graph-traversal.js</argument>
+ </arguments>
+ </configuration>
+ </execution>
+ <execution>
+ <id>js-tests</id>
--- End diff --
It uses JDK `jjs` tool to run the test contained in the 3 test files below (see `arguments`).
Currently it does not produce any output if succeeds, but it fails the build if any test fail with the stack trace and error.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #450: TINKERPOP-1489 Javascript GLV
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/450
I can take a shot at TINKERPOP-1490. I think we have community consensus on the approach if I remember correctly. I will work on it this week.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #450: Javascript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/450
Thanks @PommeVerte for getting a first look at it so fast.
About `list()` and `one()`:
- `next()` is a method exposed by the [Iterator protocol][1] which in newer versions of Ecmascript are used by [generators][2]. It could be confusing for the users, as it could give the impression that it is iterable but it isn't, users might be misguided to use it in this way: `for (let vertex of g.V())` (doesn't work because iterator protocol is sync only). Other possible names besides `one(callback)`: `getNext(callback)`, `getFirst(callback)`, `first(callback)`, `single(callback)` .
- `toList()` in JS sounded to me like a sync conversion method, ie: `const x = y.toList()`, so I went for leaving the `to` out: `list(callback)`, but maybe this is too personal...
Regarding not including the driver:
I think different `RemoteConnection` implementations could live outside of the TinkerPop repository (at least for now) as it would increase the complexity of the test harness inside TinkerPop.
[gremlin-javascript client][3] already implements most of the logic of the driver for Node.js, it would be possible to repurpose it to be a GLV `RemoteConnection` implementation but it would only be valid for Node.js (4+?).
[1]:https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Iteration_protocols#iterator
[2]:https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Iteration_protocols#With_a_generator
[3]:https://github.com/jbmusso/gremlin-javascript/blob/master/src/GremlinClient.js
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #450: TINKERPOP-1489 Javascript GLV
Posted by jbmusso <gi...@git.apache.org>.
Github user jbmusso commented on the issue:
https://github.com/apache/tinkerpop/pull/450
@jorgebay I'd be happy to help with the work on this. I recently [added partial JavaScript-GLV support to gremlin-javascript](https://github.com/jbmusso/gremlin-javascript/tree/3bdb154f41c08d5adfc65e83250eee6d57a5cab5#experimental-javascript-gremlin-language-variant). The library generates Groovy strings using [zer](https://github.com/jbmusso/zer) which is now stable/tested enough that I decided to add it as a dependency to the client. I'll soon tinker with gremlin bytecode support in zer and I think support for lambdas is easy with `Function.prototype.toString()` (ref: [mdn](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/toString)).
We could maybe add Babel dependency with nashorn/ES5 presets, then parse/transpile lambdas at runtime so people can use ES2015/2016+ lambdas in their Node.js/browsers environments (though transpilation performance could be an issue, but client-side caching/memoization could certainly help).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #450: TINKERPOP-1489 Javascript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/450#discussion_r82168010
--- Diff: gremlin-javascript/pom.xml ---
@@ -0,0 +1,132 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ 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.
+ -->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>tinkerpop</artifactId>
+ <version>3.2.3-SNAPSHOT</version>
+ </parent>
+ <artifactId>gremlin-javascript</artifactId>
+ <name>Apache TinkerPop :: Gremlin Javascript</name>
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>gremlin-core</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.codehaus.groovy</groupId>
+ <artifactId>groovy</artifactId>
+ <version>${groovy.version}</version>
+ <classifier>indy</classifier>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>tinkergraph-gremlin</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>gremlin-test</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>gremlin-server</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-log4j12</artifactId>
+ <version>${slf4j.version}</version>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+ <properties>
+ <!-- provides a way to convert maven.test.skip value to skipTests for use in skipping python tests -->
+ <maven.test.skip>false</maven.test.skip>
+ <skipTests>${maven.test.skip}</skipTests>
+ <gremlin.server.dir>${project.parent.basedir}/gremlin-server</gremlin.server.dir>
+ </properties>
+ <build>
+ <directory>${basedir}/target</directory>
+ <finalName>${project.artifactId}-${project.version}</finalName>
+ <plugins>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>exec-maven-plugin</artifactId>
+ <version>1.2.1</version>
+ <executions>
+ <execution>
+ <id>generate-javascript</id>
+ <phase>generate-test-resources</phase>
+ <goals>
+ <goal>java</goal>
+ </goals>
+ <configuration>
+ <mainClass>org.apache.tinkerpop.gremlin.javascript.GenerateGremlinJavascript</mainClass>
+ <arguments>
+ <argument>${basedir}/src/main/javascript/gremlin-javascript/process/traversal.js</argument>
+ <argument>${basedir}/src/main/javascript/gremlin-javascript/process/graph-traversal.js</argument>
+ </arguments>
+ </configuration>
+ </execution>
+ <execution>
+ <id>js-tests</id>
--- End diff --
:) mocha is nice: https://mochajs.org/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #450: TINKERPOP-1489 Javascript GLV
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/450
I've cherry picked this PR to this branch:
https://github.com/apache/tinkerpop/tree/tp32-glv
and made a commit or two to get it to compile against the tp32 branch. It's in there with #600 as well. This will allow others to more easily collaborate on the GLVs so that we can actually get these ready for merge to the release branches. We'll keep the JIRA issue open until tp32-glv is solid. In the mean time I think we can close this PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #450: TINKERPOP-1489 Javascript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/450
Great! Any help is welcome!
I have some bandwidth to dedicate on the JavaScript GLV this week, there are still several remaining items:
- `DriverRemoteConnection` implementation that can communicate with gremlin server.
- Adapt the execution pipeline to match the new async execution in TINKERPOP-1490 and expose method `promise()`
- Add Node.js runtime and run tests on mocha (ditch jjs) and create integration tests.
- Add documentation for JavaScript GLV on `gremlin-variants.asciidoc`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #450: TINKERPOP-1489 Javascript GLV
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/tinkerpop/pull/450
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #450: TINKERPOP-1489 Javascript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/450
> Note that there is a proposal for adding async iterator to JavaScript.
Nice! I wasn't aware of that, I really like the syntax!
I think the destiny of `next()` for async ops is also linked to TINKERPOP-1490, how state is managed and implemented: there are 2 possible routes for async iteration:
- We asynchronously submit the query on the first iteration and for the following iterations it just goes through the buffered results on the submitted query.
- We implement some kind of server side paging.
I think we can continue discussing this on TINKERPOP-1490.
In any case, I agree that the best approach for Javascript would be to use the async iterator protocol like you propose +1.
> [...] we could swap `var` for `let` and `const` unless we want to target earlier versions of Java8
I targeted EcmaScript 5.1 because any modern js runtime supports it, including browsers. Making the js GLV portable can be an useful feature.
Maybe we can revisit this topic when Nashorn has full ES2015 support (and probably more and more old runtimes will be deprecated).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #450: TINKERPOP-1489 Javascript GLV
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/450#discussion_r82167244
--- Diff: gremlin-javascript/pom.xml ---
@@ -0,0 +1,132 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ 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.
+ -->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <modelVersion>4.0.0</modelVersion>
+ <parent>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>tinkerpop</artifactId>
+ <version>3.2.3-SNAPSHOT</version>
+ </parent>
+ <artifactId>gremlin-javascript</artifactId>
+ <name>Apache TinkerPop :: Gremlin Javascript</name>
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>gremlin-core</artifactId>
+ <version>${project.version}</version>
+ </dependency>
+ <dependency>
+ <groupId>org.codehaus.groovy</groupId>
+ <artifactId>groovy</artifactId>
+ <version>${groovy.version}</version>
+ <classifier>indy</classifier>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>tinkergraph-gremlin</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>gremlin-test</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>gremlin-server</artifactId>
+ <version>${project.version}</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-log4j12</artifactId>
+ <version>${slf4j.version}</version>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+ <properties>
+ <!-- provides a way to convert maven.test.skip value to skipTests for use in skipping python tests -->
+ <maven.test.skip>false</maven.test.skip>
+ <skipTests>${maven.test.skip}</skipTests>
+ <gremlin.server.dir>${project.parent.basedir}/gremlin-server</gremlin.server.dir>
+ </properties>
+ <build>
+ <directory>${basedir}/target</directory>
+ <finalName>${project.artifactId}-${project.version}</finalName>
+ <plugins>
+ <plugin>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>exec-maven-plugin</artifactId>
+ <version>1.2.1</version>
+ <executions>
+ <execution>
+ <id>generate-javascript</id>
+ <phase>generate-test-resources</phase>
+ <goals>
+ <goal>java</goal>
+ </goals>
+ <configuration>
+ <mainClass>org.apache.tinkerpop.gremlin.javascript.GenerateGremlinJavascript</mainClass>
+ <arguments>
+ <argument>${basedir}/src/main/javascript/gremlin-javascript/process/traversal.js</argument>
+ <argument>${basedir}/src/main/javascript/gremlin-javascript/process/graph-traversal.js</argument>
+ </arguments>
+ </configuration>
+ </execution>
+ <execution>
+ <id>js-tests</id>
--- End diff --
I guess `jjs` wouldn't be the standard "test runner" if this were a standalone javascript project. What would be the popular choice for a test framework in javascript these days? Do any of those produce xunit style output?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---