You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by PBGraff <gi...@git.apache.org> on 2018/01/08 21:36:28 UTC
[GitHub] tinkerpop pull request #772: TINKERPOP-1861 Modify VertexProgram Builder to ...
GitHub user PBGraff opened a pull request:
https://github.com/apache/tinkerpop/pull/772
TINKERPOP-1861 Modify VertexProgram Builder to take varargs Graphs
VertexProgram.Builder.create() now takes varargs Graphs instead of just a single Graph. This change has been propagated through all methods affected. Old API is kept but deprecated for interfaces. Implementations of these interfaces have been updated to use the new API. Possible test problems that I don't understand and build flags due to API change.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/PBGraff/tinkerpop TINKERPOP-1861
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/772.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 #772
----
commit 2a121f5a18385432c55000626114561a371ba138
Author: Graff, Philip B <ph...@...>
Date: 2018-01-08T21:26:05Z
First set of changes moving to new API
commit 48cb3dd3214050f7b54f16fa3576ec36f1196c49
Author: Graff, Philip B <ph...@...>
Date: 2018-01-08T20:22:48Z
Updates to keep old API but deprecated
commit ba2236dbfe85447265a8e56cabac46e419f665d1
Author: Graff, Philip B <ph...@...>
Date: 2018-01-08T21:16:34Z
Modifying the test
----
---
[GitHub] tinkerpop pull request #772: TINKERPOP-1861 Modify VertexProgram Builder to ...
Posted by PBGraff <gi...@git.apache.org>.
Github user PBGraff commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/772#discussion_r161387755
--- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/clustering/peerpressure/PeerPressureVertexProgram.java ---
@@ -81,7 +81,11 @@ private PeerPressureVertexProgram() {
}
@Override
- public void loadState(final Graph graph, final Configuration configuration) {
+ public void loadState(final Configuration configuration, final Graph... graphs) {
+ if (graphs.length != 1) {
+ throw new IllegalStateException("Must provide one graph to use, received " + graphs.length);
+ }
+ Graph graph = graphs[0];
--- End diff --
Done.
---
[GitHub] tinkerpop pull request #772: TINKERPOP-1861 Modify VertexProgram Builder to ...
Posted by PBGraff <gi...@git.apache.org>.
Github user PBGraff commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/772#discussion_r161387762
--- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/VertexProgram.java ---
@@ -68,7 +68,20 @@ public default void storeState(final Configuration configuration) {
* @param graph the graph that the VertexProgram will run against
* @param configuration the configuration to load the state of the VertexProgram from.
*/
+ @Deprecated
--- End diff --
Thanks, done.
---
[GitHub] tinkerpop pull request #772: TINKERPOP-1861 Modify VertexProgram Builder to ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/tinkerpop/pull/772
---
[GitHub] tinkerpop issue #772: TINKERPOP-1861 Modify VertexProgram Builder to take va...
Posted by PBGraff <gi...@git.apache.org>.
Github user PBGraff commented on the issue:
https://github.com/apache/tinkerpop/pull/772
@spmallette I'm not sure I'll come back to this exactly. I was thinking about it recently, actually, and I think just providing a `create()` which calls `create(null)` in the VertexProgramBuilder would be useful for Java users. This is a simpler change than making it varagrs and less likely to cause breaking changes and probably covers 99+% of use cases.
---
[GitHub] tinkerpop pull request #772: TINKERPOP-1861 Modify VertexProgram Builder to ...
Posted by PBGraff <gi...@git.apache.org>.
Github user PBGraff commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/772#discussion_r161387752
--- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/traversal/step/VertexComputing.java ---
@@ -53,7 +53,19 @@
* @param memory the {@link Memory} from the previous OLAP job if it exists, else its an empty memory structure.
* @return the generated vertex program instance.
*/
- public VertexProgram generateProgram(final Graph graph, final Memory memory);
+ @Deprecated
--- End diff --
Done.
---
[GitHub] tinkerpop pull request #772: TINKERPOP-1861 Modify VertexProgram Builder to ...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/772#discussion_r160840348
--- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/ranking/pagerank/PageRankVertexProgram.java ---
@@ -75,7 +75,11 @@ private PageRankVertexProgram() {
}
@Override
- public void loadState(final Graph graph, final Configuration configuration) {
+ public void loadState(final Configuration configuration, final Graph... graphs) {
+ if (graphs.length != 1) {
+ throw new IllegalStateException("Must provide one graph to use, received " + graphs.length);
+ }
+ Graph graph = graphs[0];
--- End diff --
Please add a `final` here as `graph` isn't re-assigned.
---
[GitHub] tinkerpop issue #772: TINKERPOP-1861 Modify VertexProgram Builder to take va...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/772
I add a bunch of comments - pretty minor changes mostly. The biggest thing I'm seeing right now comes from the failing Travis build:
```text
[ERROR] /home/travis/build/apache/tinkerpop/spark-gremlin/src/main/java/org/apache/tinkerpop/gremlin/spark/process/computer/traversal/strategy/optimization/SparkInterceptorStrategy.java:[47,112] cannot find symbol
symbol: method getTraversal()
location: interface org.apache.tinkerpop.gremlin.process.computer.VertexProgram
[ERROR] /home/travis/build/apache/tinkerpop/spark-gremlin/src/main/java/org/apache/tinkerpop/gremlin/spark/process/computer/traversal/strategy/optimization/SparkSingleIterationStrategy.java:[64,112] cannot find symbol
symbol: method getTraversal()
location: interface org.apache.tinkerpop.gremlin.process.computer.VertexProgram
```
Is that the problem you were alluding to in the PR description?
---
[GitHub] tinkerpop pull request #772: TINKERPOP-1861 Modify VertexProgram Builder to ...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/772#discussion_r161791620
--- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/VertexProgram.java ---
@@ -219,13 +234,28 @@ public default void workerIterationEnd(final Memory memory) {
* @param <V> The vertex program type
* @return the newly constructed vertex program
*/
+ @Deprecated
--- End diff --
you need a "@deprecated" javadoc here.
---
[GitHub] tinkerpop pull request #772: TINKERPOP-1861 Modify VertexProgram Builder to ...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/772#discussion_r160840279
--- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/clustering/peerpressure/PeerPressureVertexProgram.java ---
@@ -81,7 +81,11 @@ private PeerPressureVertexProgram() {
}
@Override
- public void loadState(final Graph graph, final Configuration configuration) {
+ public void loadState(final Configuration configuration, final Graph... graphs) {
+ if (graphs.length != 1) {
+ throw new IllegalStateException("Must provide one graph to use, received " + graphs.length);
+ }
+ Graph graph = graphs[0];
--- End diff --
Our style is to `final` all variables that can be marked as such - it seems like this one could be.
---
[GitHub] tinkerpop pull request #772: TINKERPOP-1861 Modify VertexProgram Builder to ...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/772#discussion_r160841245
--- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/traversal/step/map/VertexProgramStep.java ---
@@ -64,15 +64,15 @@ public VertexProgramStep(final Traversal.Admin traversal) {
if (this.first && this.getPreviousStep() instanceof EmptyStep) {
this.first = false;
final Graph graph = this.getTraversal().getGraph().get();
- future = this.getComputer().apply(graph).program(this.generateProgram(graph, EmptyMemory.instance())).submit();
+ future = this.getComputer().apply(graph).program(this.generateProgram(EmptyMemory.instance(), graph)).submit();
final ComputerResult result = future.get();
this.processMemorySideEffects(result.memory());
return this.getTraversal().getTraverserGenerator().generate(result, this, 1l);
} else {
final Traverser.Admin<ComputerResult> traverser = this.starts.next();
final Graph graph = traverser.get().graph();
final Memory memory = traverser.get().memory();
- future = this.generateComputer(graph).program(this.generateProgram(graph, memory)).submit();
+ future = this.getComputer().apply(graph).program(this.generateProgram(memory, graph)).submit();
--- End diff --
any reason this logic is changed? it was `generateComputer()` before and now it's `getComputer().apply()`.
---
[GitHub] tinkerpop issue #772: TINKERPOP-1861 Modify VertexProgram Builder to take va...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/772
ok - i'll just close this then. if you change your mind, we can get it reopened and work it through. thanks!
---
[GitHub] tinkerpop issue #772: TINKERPOP-1861 Modify VertexProgram Builder to take va...
Posted by PBGraff <gi...@git.apache.org>.
Github user PBGraff commented on the issue:
https://github.com/apache/tinkerpop/pull/772
@spmallette That might have occurred with my attempt to rebase on the forked project. It ended up having to do a merge from my local into my remote branch, so those commits are probably (hopefully) already on the `tp32` branch. If it's really messed up, I can close this and apply the commits to a fresh branch off of the current `tp32`. The same goes for #770 .
---
[GitHub] tinkerpop pull request #772: TINKERPOP-1861 Modify VertexProgram Builder to ...
Posted by PBGraff <gi...@git.apache.org>.
Github user PBGraff commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/772#discussion_r161387754
--- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/ranking/pagerank/PageRankVertexProgram.java ---
@@ -75,7 +75,11 @@ private PageRankVertexProgram() {
}
@Override
- public void loadState(final Graph graph, final Configuration configuration) {
+ public void loadState(final Configuration configuration, final Graph... graphs) {
+ if (graphs.length != 1) {
+ throw new IllegalStateException("Must provide one graph to use, received " + graphs.length);
+ }
+ Graph graph = graphs[0];
--- End diff --
Done.
---
[GitHub] tinkerpop pull request #772: TINKERPOP-1861 Modify VertexProgram Builder to ...
Posted by PBGraff <gi...@git.apache.org>.
Github user PBGraff commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/772#discussion_r161387742
--- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/traversal/step/map/VertexProgramStep.java ---
@@ -64,15 +64,15 @@ public VertexProgramStep(final Traversal.Admin traversal) {
if (this.first && this.getPreviousStep() instanceof EmptyStep) {
this.first = false;
final Graph graph = this.getTraversal().getGraph().get();
- future = this.getComputer().apply(graph).program(this.generateProgram(graph, EmptyMemory.instance())).submit();
+ future = this.getComputer().apply(graph).program(this.generateProgram(EmptyMemory.instance(), graph)).submit();
final ComputerResult result = future.get();
this.processMemorySideEffects(result.memory());
return this.getTraversal().getTraverserGenerator().generate(result, this, 1l);
} else {
final Traverser.Admin<ComputerResult> traverser = this.starts.next();
final Graph graph = traverser.get().graph();
final Memory memory = traverser.get().memory();
- future = this.generateComputer(graph).program(this.generateProgram(graph, memory)).submit();
+ future = this.getComputer().apply(graph).program(this.generateProgram(memory, graph)).submit();
--- End diff --
I'll revert this change.
---
[GitHub] tinkerpop issue #772: TINKERPOP-1861 Modify VertexProgram Builder to take va...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/772
> My concern is that people may still override/implement the old method and not the new one, thus yielding programs that don't execute properly
Hmm....as I look at the way it is now, you mention a valid concern. This change effectively breaks existing `VertexProgram` implementations, doesn't it? Meaning, nothing in TinkerPop calls the deprecated method anymore, so existing `VertexProgram` implementations will no longer have the old method called at runtime - it will call the empty default method instead here:
https://github.com/apache/tinkerpop/pull/772/files#diff-ca108f45ec7b5a288dfce8019f1a6813L72
Perhaps the right way to do this is to:
1. Make the new method call the deprecated one by default. In that way, existing `VertexProgram` implementations just work on upgrade. Leave the deprecated one empty.
2. Our `VertexProgram` implementations implement the new method, so they will all work without a problem. The old deprecated method should be implemented on these classes to call the new method. This way, if someone (for some weird reason) is relying on the behavior of the old method it will still work without change (you would need to add appropriate deprecation annotation/javadoc here.
3. People who write new `VertexProgram` implementations wouldn't bother with the old deprecated method - perhaps some stronger language in the javadoc would make that clear to them.
3. Write [upgrade docs](https://github.com/apache/tinkerpop/blob/tp32/docs/src/upgrade/release-3.2.x-incubating.asciidoc) to describe the new expectations around `VertexProgram` implementations and include a changelog entry - i can do that if you like on merge.
How does that sound?
btw, sorry - this is turning into a complicated PR - thanks for continuing to work through this with us.
---
[GitHub] tinkerpop pull request #772: TINKERPOP-1861 Modify VertexProgram Builder to ...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/772#discussion_r160840828
--- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/traversal/step/map/PageRankVertexProgramStep.java ---
@@ -83,17 +83,17 @@ public String toString() {
}
@Override
- public PageRankVertexProgram generateProgram(final Graph graph, final Memory memory) {
+ public PageRankVertexProgram generateProgram(final Memory memory, final Graph... graphs) {
final Traversal.Admin<Vertex, Edge> detachedTraversal = this.edgeTraversal.getPure();
- detachedTraversal.setStrategies(TraversalStrategies.GlobalCache.getStrategies(graph.getClass()));
+ detachedTraversal.setStrategies(TraversalStrategies.GlobalCache.getStrategies(graphs[0].getClass()));
--- End diff --
I've not thought this through, but I think we would only ever have one `Graph` instance in this case - should that be validated at all?
---
[GitHub] tinkerpop pull request #772: TINKERPOP-1861 Modify VertexProgram Builder to ...
Posted by PBGraff <gi...@git.apache.org>.
Github user PBGraff commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/772#discussion_r162138049
--- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/VertexProgram.java ---
@@ -219,13 +234,28 @@ public default void workerIterationEnd(final Memory memory) {
* @param <V> The vertex program type
* @return the newly constructed vertex program
*/
+ @Deprecated
--- End diff --
Done in new commit. Will push with other changes.
---
[GitHub] tinkerpop pull request #772: TINKERPOP-1861 Modify VertexProgram Builder to ...
Posted by PBGraff <gi...@git.apache.org>.
Github user PBGraff commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/772#discussion_r161387759
--- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/bulkloading/BulkLoaderVertexProgram.java ---
@@ -325,9 +325,12 @@ private Builder() {
@SuppressWarnings("unchecked")
@Override
- public BulkLoaderVertexProgram create(final Graph graph) {
- ConfigurationUtils.append(graph.configuration().subset(BULK_LOADER_VERTEX_PROGRAM_CFG_PREFIX), configuration);
- return (BulkLoaderVertexProgram) VertexProgram.createVertexProgram(graph, configuration);
+ public BulkLoaderVertexProgram create(final Graph... graphs) {
+ if (graphs.length == 0) {
+ throw new IllegalStateException("Must provide at least one graph to use");
--- End diff --
Done.
---
[GitHub] tinkerpop pull request #772: TINKERPOP-1861 Modify VertexProgram Builder to ...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/772#discussion_r160839238
--- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/VertexProgram.java ---
@@ -68,7 +68,20 @@ public default void storeState(final Configuration configuration) {
* @param graph the graph that the VertexProgram will run against
* @param configuration the configuration to load the state of the VertexProgram from.
*/
+ @Deprecated
--- End diff --
When we deprecate, the javadoc should also include something like:
```text
@deprecated As of release 3.2.8, replaced by {@link #loadState(Configuration, Graph)}
```
That syntax on the link isn't quite right because Graph is varargs, but hopefully you get the idea.
---
[GitHub] tinkerpop issue #772: TINKERPOP-1861 Modify VertexProgram Builder to take va...
Posted by PBGraff <gi...@git.apache.org>.
Github user PBGraff commented on the issue:
https://github.com/apache/tinkerpop/pull/772
I think all test problems have now been fixed. My concern is that people may still override/implement the old method and not the new one, thus yielding programs that don't execute properly. It will be indicated to them that they are using a deprecated method, however.
---
[GitHub] tinkerpop pull request #772: TINKERPOP-1861 Modify VertexProgram Builder to ...
Posted by PBGraff <gi...@git.apache.org>.
Github user PBGraff commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/772#discussion_r161387751
--- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/traversal/step/map/PageRankVertexProgramStep.java ---
@@ -83,17 +83,17 @@ public String toString() {
}
@Override
- public PageRankVertexProgram generateProgram(final Graph graph, final Memory memory) {
+ public PageRankVertexProgram generateProgram(final Memory memory, final Graph... graphs) {
final Traversal.Admin<Vertex, Edge> detachedTraversal = this.edgeTraversal.getPure();
- detachedTraversal.setStrategies(TraversalStrategies.GlobalCache.getStrategies(graph.getClass()));
+ detachedTraversal.setStrategies(TraversalStrategies.GlobalCache.getStrategies(graphs[0].getClass()));
--- End diff --
Since it's called through a step, I think you're right that there should only be one `Graph`. I will validate in the "steps" that use the `Graph`.
---
[GitHub] tinkerpop issue #772: TINKERPOP-1861 Modify VertexProgram Builder to take va...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/772
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 #772: TINKERPOP-1861 Modify VertexProgram Builder to take va...
Posted by PBGraff <gi...@git.apache.org>.
Github user PBGraff commented on the issue:
https://github.com/apache/tinkerpop/pull/772
@spmallette Thanks for the heads-up, but I don't think I'll have a chance to get to it before then. It isn't so urgent it can't wait for the next release as it's code style as opposed to a bug.
---
[GitHub] tinkerpop issue #772: TINKERPOP-1861 Modify VertexProgram Builder to take va...
Posted by PBGraff <gi...@git.apache.org>.
Github user PBGraff commented on the issue:
https://github.com/apache/tinkerpop/pull/772
I think that sounds like a reasonable plan and I would appreciate it if you can cover part 4 on merge. I'll try to get to it soon. I understand that this can get more complicated since I am proposing an API change, which I wouldn't expect to be simple, especially for something more central. Thanks.
---
[GitHub] tinkerpop issue #772: TINKERPOP-1861 Modify VertexProgram Builder to take va...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/772
@PBGraff do you think you will come back to this PR to get it finished up? or is that unlikely given your schedule?
---
[GitHub] tinkerpop issue #772: TINKERPOP-1861 Modify VertexProgram Builder to take va...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/772
Sorry - you really shouldnt have any commits from our repo in this PR - it should be all your work. The problem is that if when we look at the diff to review, it contains changes that don't pertain to your work, which makes the changes impossible to evaluate by themselves. I imagine you must have merged tp32 into the branch in your fork or something. You should have instead rebased your local changes onto our tp32 which would have re-wrote all of your commits on top of ours so that the git history made it look like you had all your changes on the head of tp32. Hope that makes sense. If you need some more git explanation, please let me know.
---
[GitHub] tinkerpop pull request #772: TINKERPOP-1861 Modify VertexProgram Builder to ...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/772#discussion_r160840162
--- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/bulkloading/BulkLoaderVertexProgram.java ---
@@ -325,9 +325,12 @@ private Builder() {
@SuppressWarnings("unchecked")
@Override
- public BulkLoaderVertexProgram create(final Graph graph) {
- ConfigurationUtils.append(graph.configuration().subset(BULK_LOADER_VERTEX_PROGRAM_CFG_PREFIX), configuration);
- return (BulkLoaderVertexProgram) VertexProgram.createVertexProgram(graph, configuration);
+ public BulkLoaderVertexProgram create(final Graph... graphs) {
+ if (graphs.length == 0) {
+ throw new IllegalStateException("Must provide at least one graph to use");
--- End diff --
Better to use a `IllegalArgumentException` in this case as the validation is on the an argument to `create()` ? I think it should be changed in all the "programs" as they all seem to validate in the same manner.
---
[GitHub] tinkerpop pull request #772: TINKERPOP-1861 Modify VertexProgram Builder to ...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/772#discussion_r160840507
--- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/traversal/step/VertexComputing.java ---
@@ -53,7 +53,19 @@
* @param memory the {@link Memory} from the previous OLAP job if it exists, else its an empty memory structure.
* @return the generated vertex program instance.
*/
- public VertexProgram generateProgram(final Graph graph, final Memory memory);
+ @Deprecated
--- End diff --
As noted above, the javadoc needs some entries given the deprecation.
---
[GitHub] tinkerpop issue #772: TINKERPOP-1861 Modify VertexProgram Builder to take va...
Posted by PBGraff <gi...@git.apache.org>.
Github user PBGraff commented on the issue:
https://github.com/apache/tinkerpop/pull/772
No worries, thanks!
---
[GitHub] tinkerpop issue #772: TINKERPOP-1861 Modify VertexProgram Builder to take va...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/772
actually - i see that there is a whole other host of errors:
```text
[ERROR] Errors:
[ERROR] org.apache.tinkerpop.gremlin.process.computer.GraphComputerTest.shouldFailWithImproperTraverserRequirements(org.apache.tinkerpop.gremlin.process.computer.GraphComputerTest)
[INFO] Run 1: PASS
[ERROR] Run 2: GraphComputerTest.shouldFailWithImproperTraverserRequirements:2448 » NullPointer
[ERROR] Run 3: GraphComputerTest.shouldFailWithImproperTraverserRequirements:2448 » NullPointer
[INFO] Run 4: PASS
[INFO] Run 5: PASS
[INFO]
[ERROR] org.apache.tinkerpop.gremlin.process.computer.GraphComputerTest.shouldSucceedWithProperTraverserRequirements(org.apache.tinkerpop.gremlin.process.computer.GraphComputerTest)
[INFO] Run 1: PASS
[ERROR] Run 2: GraphComputerTest.shouldSucceedWithProperTraverserRequirements:2426 » NullPointer
[ERROR] Run 3: GraphComputerTest.shouldSucceedWithProperTraverserRequirements:2426 » NullPointer
[INFO] Run 4: PASS
[INFO] Run 5: PASS
[INFO]
[ERROR] GroovyProgramTest$Traversals>ProgramTest.g_V_outXcreatedX_aggregateXxX_byXlangX_groupCount_programXTestProgramX_asXaX_selectXa_xX:117 » IllegalState
[ERROR] org.apache.tinkerpop.gremlin.process.traversal.step.map.ProgramTest$Traversals.g_V_outXcreatedX_aggregateXxX_byXlangX_groupCount_programXTestProgramX_asXaX_selectXa_xX(org.apache.tinkerpop.gremlin.process.traversal.step.map.ProgramTest$Traversals)
[ERROR] Run 1: ProgramTest$Traversals>ProgramTest.g_V_outXcreatedX_aggregateXxX_byXlangX_groupCount_programXTestProgramX_asXaX_selectXa_xX:117 » IllegalState
[ERROR] Run 2: ProgramTest$Traversals>ProgramTest.g_V_outXcreatedX_aggregateXxX_byXlangX_groupCount_programXTestProgramX_asXaX_selectXa_xX:117 » IllegalState
[ERROR] Run 3: ProgramTest$Traversals>ProgramTest.g_V_outXcreatedX_aggregateXxX_byXlangX_groupCount_programXTestProgramX_asXaX_selectXa_xX:117 » IllegalState
[ERROR] Run 4: ProgramTest$Traversals>ProgramTest.g_V_outXcreatedX_aggregateXxX_byXlangX_groupCount_programXTestProgramX_asXaX_selectXa_xX:117 » IllegalState
[ERROR] Run 5: ProgramTest$Traversals>ProgramTest.g_V_outXcreatedX_aggregateXxX_byXlangX_groupCount_programXTestProgramX_asXaX_selectXa_xX:117 » IllegalState
```
Are you still looking into a resolution for these?
---
[GitHub] tinkerpop issue #772: TINKERPOP-1861 Modify VertexProgram Builder to take va...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/772
@PBGraff i'm not sure why i didn't notice this before, but i see a bunch of commits here that aren't yours. Did something get messed up in your fork somehow?
---
[GitHub] tinkerpop issue #772: TINKERPOP-1861 Modify VertexProgram Builder to take va...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/772
@PBGraff i just thought i'd reach out to let you know that we do have a release coming up soon - aimed for first week of april (though we'd probably look to stop making code changes in preparation for that on March 23). Do you think there is any chance you might have time to come back to this PR in time for that release? if not, that's fine, i just wanted you to be aware that a release was upcoming and if this issue was important to you and you needed it for your work in the next release, that time is coming up. thanks
---