Skip to content

Conversation

@FlorianHockmann
Copy link
Member

https://issues.apache.org/jira/browse/TINKERPOP-1868

This adds the Inject step to GraphTraversalSource for Gremlin.Net. While working on this I also noticed that the interface wasn't optimal for the existing GraphTraversal Inject step as it accepted a params object[] injections although it should only except injections of the generic type E. So I also fixed that.

Method signature before:

public GraphTraversal< S , E > Inject (params object[] injections)

and after:

public GraphTraversal< S , E > Inject (params E[] injections)

Now, a traversal like the following isn't possible any more in Gremlin.Net:

g.V().Values<string>("name").Inject(1).ToList();

as Inject now only accepts strings here (just like Gremlin-Java does).

@FlorianHockmann
Copy link
Member Author

$ docker/build.sh -t -i
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO] 
[INFO] Apache TinkerPop .................................. SUCCESS [1:14.896s]
[INFO] Apache TinkerPop :: Gremlin Shaded ................ SUCCESS [17.284s]
[INFO] Apache TinkerPop :: Gremlin Core .................. SUCCESS [1:00.729s]
[INFO] Apache TinkerPop :: Gremlin Test .................. SUCCESS [8.073s]
[INFO] Apache TinkerPop :: Gremlin Groovy ................ SUCCESS [2:33.551s]
[INFO] Apache TinkerPop :: Gremlin Groovy Test ........... SUCCESS [5.123s]
[INFO] Apache TinkerPop :: TinkerGraph Gremlin ........... SUCCESS [3:54.724s]
[INFO] Apache TinkerPop :: Gremlin Benchmark ............. SUCCESS [6.655s]
[INFO] Apache TinkerPop :: Gremlin Driver ................ SUCCESS [1:15.868s]
[INFO] Apache TinkerPop :: Neo4j Gremlin ................. SUCCESS [1.776s]
[INFO] Apache TinkerPop :: Gremlin Server ................ SUCCESS [9:36:46.546s]
[INFO] Apache TinkerPop :: Gremlin Python ................ SUCCESS [1:38.416s]
[INFO] Apache TinkerPop :: Gremlin.Net ................... SUCCESS [2.961s]
[INFO] Apache TinkerPop :: Gremlin.Net - Source .......... SUCCESS [58.269s]
[INFO] Apache TinkerPop :: Gremlin.Net - Tests ........... SUCCESS [52.578s]
[INFO] Apache TinkerPop :: Hadoop Gremlin ................ SUCCESS [8:02.139s]
[INFO] Apache TinkerPop :: Spark Gremlin ................. SUCCESS [17:15.718s]
[INFO] Apache TinkerPop :: Giraph Gremlin ................ SUCCESS [3:16:25.204s]
[INFO] Apache TinkerPop :: Gremlin Console ............... SUCCESS [4:09.178s]
[INFO] Apache TinkerPop :: Gremlin Archetype ............. SUCCESS [0.261s]
[INFO] Apache TinkerPop :: Archetype - TinkerGraph ....... SUCCESS [25.908s]
[INFO] Apache TinkerPop :: Archetype - Server ............ SUCCESS [15.679s]
[INFO] Apache TinkerPop :: Archetype - DSL ............... SUCCESS [4.216s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 13:37:42.997s
[INFO] Finished at: Sun Jan 14 12:37:29 UTC 2018
[INFO] Final Memory: 169M/552M
[INFO] ------------------------------------------------------------------------

VOTE: +1

@spmallette
Copy link
Contributor

If this is a breaking change for .NET users in some way, it would be good to write something in the upgrade documentation about it. A changelog entry would also be good (either way).

@FlorianHockmann
Copy link
Member Author

I don't think that it is a breaking change as it only prevents users from sending traversals with invalid arguments for the inject step to the server which should cause a runtime error on the server side at the moment. With this change, this runtime error would now be a compile time error.

@spmallette
Copy link
Contributor

ah - ok. i get what is happening now. thanks for clarifying. in that case:

VOTE +1

{
<% if (method.parameters.contains("params ")) {
%> var args = new List<object>(<%= method.paramNames.init().size() %> + <%= method.paramNames.last() %>.Length) {<%= method.paramNames.init().join(", ") %>};
%> var args = new List< <%= method.argsListType %> >(<%= method.paramNames.init().size() %> + <%= method.paramNames.last() %>.Length) {<%= method.paramNames.init().join(", ") %>};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do something about the extra spacing?

Copy link
Contributor

@spmallette spmallette Jan 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 - would love to give a cleanup to all our templates. they all have little spacing issues like this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, those spaces shouldn't be there and I just found a way to remove them: Variables can be used in a Groovy template in the form of $variable which avoids the problem with the double < as <<%= variable %>> doesn't work.
I pushed a commit that should remove all those unnecessary spaces from the generated classes in Gremlin.Net (at least I hope that I got all).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, nice!
I've reviewed the end result and looks good.

@jorgebay
Copy link
Contributor

Looks great, I've added comment / question above but its just a nit.

VOTE +1

@asfgit asfgit merged commit 535b309 into tp32 Jan 21, 2018
@asfgit asfgit deleted the TINKERPOP-1868 branch April 14, 2018 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants