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 2017/08/17 15:40:46 UTC
[GitHub] tinkerpop pull request #695: TINKERPOP-1489 JavaScript GLV
GitHub user jorgebay opened a pull request:
https://github.com/apache/tinkerpop/pull/695
TINKERPOP-1489 JavaScript GLV
https://issues.apache.org/jira/browse/TINKERPOP-1489
Submitting the JavaScript for review to merge into tp32 after 3.2.6 code freeze and release.
Changes since #626:
- Rebased.
- Adapted code generation to use groovy templates (similar to #608 for Python).
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/tinkerpop TINKERPOP-1489
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/695.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 #695
----
commit fbeae06cad99e4d3f5ce1f3f3fbe02641cb783d1
Author: Jorge Bay Gondra <jo...@gmail.com>
Date: 2016-10-05T14:14:46Z
Javascript GLV
commit ba0f0ac56c936ba848f7bbe460ae9772128f3ae1
Author: Jorge Bay Gondra <jo...@gmail.com>
Date: 2016-10-07T10:01:30Z
Export enums and fix TraversalStrategies#applyStrategies()
commit 673040d5d50e4619c375b1cbcc59e65c344a98c0
Author: Jorge Bay Gondra <jo...@gmail.com>
Date: 2016-10-18T14:22:23Z
Graph, traversalStrategies and graph as Traversal properties
Expose those properties as in the Python GLV and the Java implementation. Previously, those properties where exposed using the private notation (ie: _graph).
commit 29f43b0344116d05e4d9b38a85acc73c203642a8
Author: Jorge Bay Gondra <jo...@gmail.com>
Date: 2016-10-19T14:03:45Z
Filter out __() static method
commit 3769fdd11a635894a53de53b16f32d164ca38c0d
Author: Jorge Bay Gondra <jo...@gmail.com>
Date: 2016-11-03T13:34:28Z
Use null as empty result
commit 14c0209a514993ba706c30daccc3ea09527c014a
Author: Jorge Bay Gondra <jo...@gmail.com>
Date: 2016-11-16T12:41:46Z
Parse Edge and VertexProperty properties
To follow the decision around TINKERPOP-1474 for the GLV to parse properties.
commit eab70fdc0433ba540fc3a71afd4e651e43d92875
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2017-05-18T16:47:48Z
TINKERPOP-1489 Cleaned up pom
Removed some weird characters in license and bumped version to 3.2.5-SNAPSHOT
commit fdcc5affc37fae38c695439f2e973d9fcf436acb
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2017-05-18T16:48:27Z
TINKERPOP-1489 Regenerated traversal.js
which added Pick.
commit dfcb46f859c6b66e9a5f56a210a8148147de762e
Author: Jorge Bay Gondra <jo...@gmail.com>
Date: 2017-06-06T13:05:14Z
Update Javascript GLV
Address feedback and provide maven integration:
- Reorganize gremlin-javascript into node.js project
- Simplify javascript code generators
- Generate package.json to match project version
- Introduce Promise factory for third-party promise library integration (ie: bluebird)
- Use toList() and next() traversal methods
- Include Remote connection implementation
- Fix enum casing
- Use Maven nodejs plugin
- .gitignore and .npmignore at gremlin-javascript level
- Run integration tests using a gremlin-server instance
- Add gremlin-javascript doc in gremlin-variants.asciidoc
commit 887d3ae9208eb62d5e9940d0b5976abceddd7271
Author: Jorge Bay Gondra <jo...@gmail.com>
Date: 2017-08-17T11:35:47Z
Bump to 3.2.6 in gremlin-javascript
commit 043be33f5ca878c94b6ad551b55bd9d280706024
Author: Jorge Bay Gondra <jo...@gmail.com>
Date: 2017-08-17T15:28:32Z
Js GLV: Use Groovy templates for generation
----
---
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 #695: TINKERPOP-1489 JavaScript GLV
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/695
I mostly cared about nashorn to achieve testing as we did in python. I sense that most folks won't want to run on nashorn itself, so perhaps direct support of nashorn doesn't really matter especially if we come up with a better way to test. I'd rather not complicate gremlin-javascript with multiple runtimes - GLVs require enough effort to support as it is.
btw, i felt like this lib got me pretty far in making npm work on nashorn:
https://github.com/nodyn/jvm-npm
wasn't too bad, but got tripped up on the pathing as it didn't seem to want to run in a context that didn't have the js files in the same root directory as where you were running the scriptengine from. I couldn't quite figure out how to make that work. Again, it probably doesn't make sense to chase that angle anymore though. I'd rather just get testing figured out for GLVs in general.
> We can define a set of given-when-then statements (in doc), which we can make sure each GLV implements in actual tests.
if we can write them in a doc, then it would seem we could write them as cucumber tests ( https://cucumber.io/ ) that could actually be executed somehow. that's been the general plan i've had anyway. if you have any other ideas, please let me know - i'll probably start playing around with this idea today or get started on it fresh next week.
---
[GitHub] tinkerpop pull request #695: 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/695#discussion_r159677279
--- Diff: gremlin-javascript/pom.xml ---
@@ -0,0 +1,366 @@
+<!--
+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.7-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.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>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+ <properties>
+ <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.apache.maven.plugins</groupId>
+ <artifactId>maven-install-plugin</artifactId>
+ <configuration>
+ <skip>true</skip>
+ </configuration>
+ </plugin>
+ <plugin>
+ <!--
+ Use gmavenplus-plugin to:
+ - Generate js sources
+ - Start and stop gremlin server for integration tests
+ -->
+ <groupId>org.codehaus.gmavenplus</groupId>
+ <artifactId>gmavenplus-plugin</artifactId>
+ <dependencies>
+ <dependency>
+ <groupId>log4j</groupId>
+ <artifactId>log4j</artifactId>
+ <version>1.2.17</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>gremlin-server</artifactId>
+ <version>${project.version}</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.codehaus.groovy</groupId>
+ <artifactId>groovy-ant</artifactId>
+ <version>${groovy.version}</version>
+ </dependency>
+ </dependencies>
+ <executions>
+ <execution>
+ <id>generate-javascript</id>
+ <phase>generate-sources</phase>
+ <goals>
+ <goal>execute</goal>
+ </goals>
+ <configuration>
+ <scripts>
+ <script><![CDATA[
+import org.apache.tinkerpop.gremlin.jsr223.CoreImports
+import org.apache.tinkerpop.gremlin.process.traversal.TraversalSource
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource
+import org.apache.tinkerpop.gremlin.process.traversal.P
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__
+import java.lang.reflect.Modifier
+
+def toJsMap = ["in": "in_",
+ "from": "from_"]
+
+def toJs = { symbol -> toJsMap.getOrDefault(symbol, symbol) }
+
+def decapitalize = {
+ String string = it;
+ if (string == null || string.length() == 0) {
+ return string;
+ }
+ def c = string.toCharArray();
+ c[0] = Character.toLowerCase(c[0]);
+ return new String(c);
+}
+
+def determineVersion = {
+ def env = System.getenv()
+ def mavenVersion = env.containsKey("TP_RELEASE_VERSION") ? env.get("JS_RELEASE_VERSION") : '${project.version}'
+ return mavenVersion.replace("-SNAPSHOT", "-alpha1")
+}
+
+def binding = ["enums": CoreImports.getClassImports()
+ .findAll { Enum.class.isAssignableFrom(it) }
+ .sort { a, b -> a.getSimpleName() <=> b.getSimpleName() },
+ "pmethods": P.class.getMethods().
+ findAll { Modifier.isStatic(it.getModifiers()) }.
+ findAll { P.class.isAssignableFrom(it.returnType) }.
+ collect { it.name }.
+ unique().
+ sort { a, b -> a <=> b },
+ "sourceStepMethods": GraphTraversalSource.getMethods(). // SOURCE STEPS
+ findAll { GraphTraversalSource.class.equals(it.returnType) }.
+ findAll {
+ !it.name.equals("clone") &&
+ // Use hardcoded name to be for forward-compatibility
+ !it.name.equals("withBindings") &&
+ !it.name.equals(TraversalSource.Symbols.withRemote) &&
+ !it.name.equals(TraversalSource.Symbols.withComputer)
+ }.
+ collect { it.name }.
+ unique().
+ sort { a, b -> a <=> b },
+ "sourceSpawnMethods": GraphTraversalSource.getMethods(). // SPAWN STEPS
+ findAll { GraphTraversal.class.equals(it.returnType) }.
+ collect { it.name }.
+ unique().
+ sort { a, b -> a <=> b },
+ "graphStepMethods": GraphTraversal.getMethods().
+ findAll { GraphTraversal.class.equals(it.returnType) }.
+ findAll { !it.name.equals("clone") && !it.name.equals("iterate") }.
+ collect { it.name }.
+ unique().
+ sort { a, b -> a <=> b },
+ "anonStepMethods": __.class.getMethods().
+ findAll { GraphTraversal.class.equals(it.returnType) }.
+ findAll { Modifier.isStatic(it.getModifiers()) }.
+ findAll { !it.name.equals("__") && !it.name.equals("start") }.
+ collect { it.name }.
+ unique().
+ sort { a, b -> a <=> b },
+ "toJs": toJs,
+ "version": determineVersion(),
+ "decapitalize": decapitalize]
+
+def engine = new groovy.text.GStringTemplateEngine()
+def graphTraversalTemplate = engine.createTemplate(new File('${project.basedir}/glv/GraphTraversalSource.template')).make(binding)
+def graphTraversalFile = new File('${project.basedir}/src/main/javascript/gremlin-javascript/lib/process/graph-traversal.js')
+graphTraversalFile.newWriter().withWriter{ it << graphTraversalTemplate }
+
+def traversalTemplate = engine.createTemplate(new File('${project.basedir}/glv/TraversalSource.template')).make(binding)
+def traversalFile = new File('${project.basedir}/src/main/javascript/gremlin-javascript/lib/process/traversal.js')
+traversalFile.newWriter().withWriter{ it << traversalTemplate }
+
+def packageJsonTemplate = engine.createTemplate(new File('${project.basedir}/glv/PackageJson.template')).make(binding)
+def packageJsonFile = new File('${project.basedir}/src/main/javascript/gremlin-javascript/package.json')
+packageJsonFile.newWriter().withWriter{ it << packageJsonTemplate }
+]]> </script>
+ </scripts>
+ </configuration>
+ </execution>
+ <execution>
+ <id>gremlin-server-start</id>
+ <phase>pre-integration-test</phase>
+ <goals>
+ <goal>execute</goal>
+ </goals>
+ <configuration>
+ <properties>
+ <property>
+ <name>skipTests</name>
+ <value>${skipTests}</value>
+ </property>
+ <property>
+ <name>python</name>
+ <value>false</value>
+ </property>
+ <property>
+ <name>gremlinServerDir</name>
+ <value>${gremlin.server.dir}</value>
+ </property>
+ <property>
+ <name>settingsFile</name>
+ <value>${gremlin.server.dir}/src/test/resources/org/apache/tinkerpop/gremlin/driver/remote/gremlin-server-integration.yaml</value>
+ </property>
+ <property>
+ <name>executionName</name>
+ <value>${project.name}</value>
+ </property>
+ <property>
+ <name>projectBaseDir</name>
+ <value>${project.basedir}</value>
+ </property>
+ </properties>
+ <scripts>
+ <script>${gremlin.server.dir}/src/test/scripts/test-server-start.groovy</script>
+ </scripts>
+ </configuration>
+ </execution>
+ <execution>
+ <id>gremlin-server-stop</id>
+ <phase>post-integration-test</phase>
+ <goals>
+ <goal>execute</goal>
+ </goals>
+ <configuration>
+ <properties>
+ <property>
+ <name>skipTests</name>
+ <value>${skipTests}</value>
+ </property>
+ <property>
+ <name>executionName</name>
+ <value>${project.name}</value>
+ </property>
+ </properties>
+ <scripts>
+ <script>${gremlin.server.dir}/src/test/scripts/test-server-stop.groovy</script>
+ </scripts>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-clean-plugin</artifactId>
+ <version>3.0.0</version>
+ <executions>
+ <execution>
+ <goals>
+ <goal>clean</goal>
+ </goals>
+ </execution>
+ </executions>
+ </plugin>
+ <plugin>
+ <groupId>com.github.eirslett</groupId>
+ <artifactId>frontend-maven-plugin</artifactId>
+ <version>1.4</version>
+ <executions>
+ <execution>
+ <id>install node and npm</id>
+ <phase>generate-test-resources</phase>
+ <goals>
+ <goal>install-node-and-npm</goal>
+ </goals>
+ </execution>
+ <execution>
+ <id>npm install</id>
+ <phase>generate-test-resources</phase>
+ <goals>
+ <goal>npm</goal>
+ </goals>
+ <configuration>
+ <arguments>install</arguments>
+ </configuration>
+ </execution>
+ <execution>
+ <id>npm test</id>
+ <phase>integration-test</phase>
+ <goals>
+ <goal>npm</goal>
+ </goals>
+ <configuration>
+ <arguments>test</arguments>
+ <failOnError>true</failOnError>
+ </configuration>
+ </execution>
+ <execution>
+ <id>npm test gherkin features</id>
+ <phase>integration-test</phase>
+ <goals>
+ <goal>npm</goal>
+ </goals>
+ <configuration>
+ <arguments>run-script features</arguments>
+ <failOnError>true</failOnError>
+ </configuration>
+ </execution>
+ </executions>
+ <configuration>
+ <skip>${skipIntegrationTests}</skip>
+ <workingDirectory>src/main/javascript/gremlin-javascript</workingDirectory>
+ <nodeVersion>v4.8.3</nodeVersion>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+ <profiles>
+ <!--
+ Provides a way to deploy the gremlin-javascript GLV to npm. This cannot be part of the standard maven execution
+ because npm does not have a staging environment like sonatype for releases. As soon as the release is
+ published it is public. In our release workflow, deploy occurs prior to vote on the release and we can't
+ make this stuff public until the vote is over.
+ -->
+ <profile>
+ <id>glv-javascript-deploy</id>
+ <activation>
+ <activeByDefault>false</activeByDefault>
+ </activation>
+ <build>
+ <plugins>
--- End diff --
is there any special configuration required in the release manager's local environment for this to work? Either way, It would be a massive help if you could provide a "Javascript Environment" section in the dev docs here:
http://tinkerpop.apache.org/docs/current/dev/developer/#system-configuration
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/695
Great! It would be really nice to have a working `GremlinJavaScriptScriptEngine`!
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/695
I've updated the travis commands to include a `mvn clean install -DskipTests` before running GLV tests, GLV jobs take now 7 and 5 minutes respectively which is still not much.
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/695
Hi @jorgebay - sorry - i just haven't had time to focus on this yet. I'll try to dig into it next week. Thanks for jamming on this and getting it ready for merging.
---
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 #695: TINKERPOP-1489 JavaScript GLV
Posted by jbmusso <gi...@git.apache.org>.
Github user jbmusso commented on the issue:
https://github.com/apache/tinkerpop/pull/695
Finally found some time. Wew.
Well, this PR is very well crafted - well done! My comments are only minor:
* About `utils.toPromise` - if I understand you well, you want a dual callback/promise API for most async functions?
* In `graph-traversal.js`, most methods of `GraphTraversalSource` and `GraphTraversal` and function attached to `statics` could maybe be dynamically created from an array of method names and dynamically added to these classes/object. I don’t know what would be the performance implication of this, but I don’t think there should be any unless V8 really can’t figure out what’s going on when parsing that file. Hopefully it's smart and figures out that the class is not changing. That’d lower the file size and help maintainability a lot.
* ES6, most likely friendlier for more recent versions of V8 and supported by Node.js v4+ (see [Node green](http://node.green)):
* `arguments` is deprecated and is replaced by `...args` for variadic functions
* most `function` keywords could be replaced by arrow functions (lexical scoping and/or concise syntax). I tend to keep `function` for top-level functions, and use fat arrows everywhere even when `this` binding isn't needed (ie. callback w/o `this`)
* `array.splice(0)` could be replaced by `const copy = [...original]`
* `func.apply(null, arguments)` could be replaced by `func(...args)` when first argument `this` value is indeed meant to be `null`
* `package.json`: `./node_modules/.bin` is added to the `$PATH` by `npm` or `yarn`, so we could just use `"test": "mocha test/unit test/integration -t 5000"`. Yay npm!
I can fork and push 4 distinct commits for this if you want, so this can be cherry-picked.
A more major update would be to author in ES6/7/8 and add a transpilation step, so all runtime could use code that they can optimize best. Using babel with [babel-preset-env](https://www.npmjs.com/package/babel-preset-env) combined with [postinstall-build](https://www.npmjs.com/package/postinstall-build) is an option. This will ensure that latest versions of Node.js use mostly non-transpiled code, while older versions automatically transpile what is strictly needed. That would make things more performant for latest versions of Node.js, since V8 optimizes a lot for new syntax, while still making the GLV compatible for older versions of Node.js. The nice thing is that the build step is automatically handled at install time by npm, so no extra maven coding should be required. I think such approach could be added in future releases and is out of scope today.
Also, I'm ok to transfer/donate the "gremlin" package name to Apache TinkerPop so this can be published under this name.
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/695
I'm getting the same failure, but I don't understand where the requirement to a newer maven is coming from within `frontend-maven-plugin`.
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/695
I've removed support for Nashorn as part of #626 (see first comment). JavaScript engines don't provide a standard way to deal with and import modules, so supporting with multiple runtimes is usually a pain (unlike other languages like Python that has the `import` system built-in).
If to support `GremlinJavaScriptScriptEngine` we need to support Nashorn on the GLV, we should try to split the GLV and the JS script engine in 2 separate tickets and try to tackle those separately.
Having a JavaScript GLV would enable users to write their traversals in js, making it easier for users from the js community to use Gremlin. On the other side, supporting js lambdas is a more marginal use case.
> I think the need to figure out the language agnostic way to test GLVs is becoming more important
I think testing a GLV (bytecode generation, websocket connection, type mapping, ...) can't be implemented in a runtime agnostic way because the whole concepts are defined by the runtime. I think we can define a set of behaviours that the GLV must adhere. We can define a set of given-when-then statements (in doc), which we can make sure each GLV implements in actual tests.
On the other hand, I think testing a script engine can be generalized into a common suite (like ScriptEngineLambdaTest) plus specialized behaviour tests per language.
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/695
VOTE +1
`mvn clean install -DskipIntegrationTests=false` passes.
API summary:
- All methods are generated using groovy template files.
- Naming conventions for js are very similar to java, so no significant changes there from the java one (except `in() and `from()` which are reserved keywords).
- `toList()` and `next()` are used, in the same way as the rest of the GLVs, except that it causes async execution (no blocking API is provided, as it won't be usable in Node.js): `next()` returns an async iterator and `toList()` a promise that gets fulfilled with an `Array`.
Usage samples:
```js
const vertices = await g.V().toList();
```
```js
for await (const vertex of g.V()) {
console.log(vertex.label);
}
```
---
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 #695: TINKERPOP-1489 JavaScript GLV
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/695
I think the need to figure out the language agnostic way to test GLVs is becoming more important. I was pretty close to getting nashorn to execute gremlin-javascript natively, but there's weird pathing issue and npm integration issues that just aren't making it easy (possible). I tried to hack my way around the pathing problems and while i was making progress it was just getting ugly. I'll need to think about the language agnostic approach a bit before I worry about trying to go any further in this direction.
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/695
All tests pass with `docker/build.sh -t -n -i`
VOTE +1
---
[GitHub] tinkerpop pull request #695: 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/695#discussion_r159676150
--- Diff: gremlin-javascript/pom.xml ---
@@ -0,0 +1,366 @@
+<!--
+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.7-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.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>
+ <scope>test</scope>
+ </dependency>
+ </dependencies>
+ <properties>
+ <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.apache.maven.plugins</groupId>
+ <artifactId>maven-install-plugin</artifactId>
+ <configuration>
+ <skip>true</skip>
+ </configuration>
+ </plugin>
+ <plugin>
+ <!--
+ Use gmavenplus-plugin to:
+ - Generate js sources
+ - Start and stop gremlin server for integration tests
+ -->
+ <groupId>org.codehaus.gmavenplus</groupId>
+ <artifactId>gmavenplus-plugin</artifactId>
+ <dependencies>
+ <dependency>
+ <groupId>log4j</groupId>
+ <artifactId>log4j</artifactId>
+ <version>1.2.17</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.tinkerpop</groupId>
+ <artifactId>gremlin-server</artifactId>
+ <version>${project.version}</version>
+ <scope>runtime</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.codehaus.groovy</groupId>
+ <artifactId>groovy-ant</artifactId>
+ <version>${groovy.version}</version>
+ </dependency>
+ </dependencies>
+ <executions>
+ <execution>
+ <id>generate-javascript</id>
+ <phase>generate-sources</phase>
+ <goals>
+ <goal>execute</goal>
+ </goals>
+ <configuration>
+ <scripts>
+ <script><![CDATA[
--- End diff --
can you please move embedded scripts to files as we did with the other GLVs?
https://github.com/apache/tinkerpop/blob/tp32/gremlin-python/pom.xml#L483
Makes the pom a bit easier to read.
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by jbmusso <gi...@git.apache.org>.
Github user jbmusso commented on the issue:
https://github.com/apache/tinkerpop/pull/695
Quick update - I plan to check this PR this weekend.
---
[GitHub] tinkerpop pull request #695: 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/695
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/695
Rebased.
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/695
I've pushed 2 commits to address the issues highlighted in the review.
I got rid of the promise factory and I'm using rest parameters and spread syntax (instead of `parseArgs()`) for `GraphTraversal` and `GraphTraversalSource` methods.
In order to ease up maintenance of the GLV, I've bumped the min version supported of the runtime to Node.js 6, as Node.js 4 is [reaching EOL in April](https://github.com/nodejs/Release) and it didn't support several ES2015 features (rest parameters being one of them).
---
[GitHub] tinkerpop pull request #695: 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/695#discussion_r159674939
--- Diff: gremlin-javascript/src/main/javascript/gremlin-javascript/package.json ---
@@ -0,0 +1,39 @@
+{
+ "name": "gremlin-javascript",
+ "version": "3.2.7-alpha1",
--- End diff --
should be 3.2.8-alpha1 - at this point. though, we've been using "release candidate" suffix for other GLVs as in "3.2.8-rc1" - perhaps we should continue in that vein?
---
[GitHub] tinkerpop pull request #695: 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/695#discussion_r160120074
--- Diff: gremlin-javascript/src/main/javascript/gremlin-javascript/package.json ---
@@ -0,0 +1,39 @@
+{
+ "name": "gremlin-javascript",
+ "version": "3.2.7-alpha1",
--- End diff --
I've used `alpha` to avoid messing up in case it was accidentally pushed while testing the pull request.
The version suffix is added in the `determineVersion()` method of the `generation.groovy` file.
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/695
gremlin-javascript seems to build ok for me locally, but not on docker. claims it needs a maven upgrade to 3.1.0 for the npm plugin to work. do you see the same thing @jorgebay ?
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/695
I rebased this today and added the nashorn scriptengine implementation for `GremlinScriptEngine`. I'd like to get us running tests with js the way we do with python since it's jsr223 compliant. I guess I'll keep working toward that goal this week - i'd propose to hold-off on merging this until that is done.
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/695
this is excellent. i agree that we should look to merge this for 3.2.8/3.3.2. as we hopefully release 3.2.7/3.3.1 we should see gremlin-js come into an official state in the first few months of 2018.
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/695
I'm posting this to all open PRs of current relevance - this PR needs to be rebased against the branch it is targeted against given a broken python dependency that was published to pypi a day or so ago. I've pushed a fix on tp32 and master at this point and David Brown is working on getting an issue raised with the project that initiated the problem.
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/695
What a great time of the year to review this pr! 😄
---
[GitHub] tinkerpop pull request #695: 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/695#discussion_r159675592
--- Diff: gremlin-javascript/pom.xml ---
@@ -0,0 +1,366 @@
+<!--
+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.7-SNAPSHOT</version>
--- End diff --
should be 3.2.8-SNAPSHOT at this point
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/695
> About `utils.toPromise` - if I understand you well, you want a dual callback/promise API for most async functions?
Just promise-based API, no callback-based API. `utils.toPromise()` its a way to support different "promise factories", the way promises are created in the driver. It's an idea to support any `Promise` API (like bluebird), but it's not really needed... maybe adds unnecessary complexity to the code? Do you think it makes more sense to remove it?
> In `graph-traversal.js`, most methods of `GraphTraversalSource` and `GraphTraversal` and function attached to `statics` [...]
I like the idea of having the methods defined in code to support code completion on IDEs like VS Code and WebStorm, given that it's hard to remember all the traversal methods by heart. Maybe we can reduce the size and complexity of the generated code by using something like:
Instead of (current):
```javascript
const statics = {};
statics.V = function (args) {
const g = new GraphTraversal(null, null, new Bytecode());
return g.V.apply(g, arguments);
};
statics.addE = function (args) {
const g = new GraphTraversal(null, null, new Bytecode());
return g.addE.apply(g, arguments);
};
```
Use:
```javascript
const statics = {
V: (...args) => callOnEmptyTraversal('V', args),
addE: (...args) => callOnEmptyTraversal('addE', args),
// ...
};
```
wdyt?
I'll start working on the other suggestions (ES6).
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/695
I've implemented the support files for the gherkin test suite.
Thanks to the test suite, I've found and fixed some bugs that were part of the original implementation.
`mvn clean install -pl :gremlin-javascript -DskipIntegrationTests=false` run the mocha based tests and the cucumber-based tests. I've included it on TravisCI also.
```
328 scenarios (22 skipped, 306 passed)
```
It's ready to be reviewed!
I would like the JavaScript GLV to be part of the next release cycle (3.2.8 / 3.3.2). This GLV have been sidelined several times since the original pull request #450 (Oct 2016!)...
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/695
Thanks @dkuppitz for looking into the maven issue!
I'll rebase it and add a `g:T` deserializer.
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/695
I've just noticed that there isn't a `g:T` deserializer, I'll add it in the next days.
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/695
Just a note for those following this ticket's progress. I'm in the process of doing TINKERPOP-1784 which will add a language agnostic test suite using gherkin defined feature files. That should allow us to test all GLVs in a nice way.
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by jorgebay <gi...@git.apache.org>.
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/695
I've addressed the comments made by @spmallette:
- I've included sections on the `development-environment.asciidoc` file for js environment and info for the release managers.
- Moved generation to a groovy file.
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by jbmusso <gi...@git.apache.org>.
Github user jbmusso commented on the issue:
https://github.com/apache/tinkerpop/pull/695
All good to me!
VOTE +1
---
[GitHub] tinkerpop issue #695: TINKERPOP-1489 JavaScript GLV
Posted by jbmusso <gi...@git.apache.org>.
Github user jbmusso commented on the issue:
https://github.com/apache/tinkerpop/pull/695
According to https://github.com/petkaantonov/bluebird/issues/1026, users should be able to just patch the global `Promise` object in their application with:
```javascript
global.Promise = require("bluebird");
```
I am unsure about other Promise libraries but I believe this approach should work as long as they're Promise/A+ standard compliant. Maybe we could also give it more thoughts and see for other ways to handle this in a future release, but I think it's worth making the code simpler at this point. I also feel it can/should be done at the application level.
For Traversal methods, I didn't think about IDEs and you're absolutely right about code completion. I think your proposed approach with `callOnEmptyTraversal` works best. This makes me think that I'm pretty sure people will want typed traversals with TypeScript or FlowType soon (thinking about https://github.com/DefinitelyTyped/DefinitelyTyped here for example).
---