[GitHub] tinkerpop pull request #953: TINKERPOP-1972 Fix inject step in Gremlin.Net

2018-10-05 Thread FlorianHockmann
GitHub user FlorianHockmann opened a pull request: https://github.com/apache/tinkerpop/pull/953 TINKERPOP-1972 Fix inject step in Gremlin.Net https://issues.apache.org/jira/browse/TINKERPOP-1972 We had two issues with the `inject` step in Gremlin.Net. The first one

[GitHub] tinkerpop pull request #947: TINKERPOP-2055 Support NaN and Infinity in Grap...

2018-10-04 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/947#discussion_r222751451 --- Diff: gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/graphson-test.js --- @@ -46,6 +46,30 @@ describe('GraphSONReader

[GitHub] tinkerpop issue #928: TINKERPOP-2015 Expose WebSocket configuration in Greml...

2018-10-04 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/928 @deejvince: Did you see [the integration test](https://github.com/apache/tinkerpop/blob/master/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Driver/GremlinClientTests.cs#L233) added

[GitHub] tinkerpop issue #928: TINKERPOP-2015 Expose WebSocket configuration in Greml...

2018-10-01 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/928 I also wanted to implement it like that as first, but unfortunately the [`ClientWebSocket.Options` property](https://docs.microsoft.com/dotnet/api

[GitHub] tinkerpop issue #928: TINKERPOP-2015 Expose WebSocket configuration in Greml...

2018-09-21 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/928 Yes, that should be possible. With this PR, you can provide a delegate to configure the ClientWebSocketOptions which should also include custom headers. ---

[GitHub] tinkerpop issue #925: TINKERPOP-2026 Make closing of connections more robust...

2018-09-21 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/925 No, there is no reason for not merging this. I'm just currently on vacation. So, I can't do it myself right now. But if you or anyone else wants to merge this, then feel free to do that. ---

[GitHub] tinkerpop issue #915: Tinkerpop 1913-Followup

2018-09-18 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/915 I'm currently out of office but I just gave this a quick look and my review comments seem to be mostly addressed. So: VOTE +1 ---

[GitHub] tinkerpop issue #903: Gremlin.Net: Add ConnectionPool min and max sizes TINK...

2018-09-11 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/903 > looks like you are focused on getting this one merged No, I don't really mind waiting for TINKERPOP-1775 and implementing both together. I just had two motivations for merg

[GitHub] tinkerpop issue #903: Gremlin.Net: Add ConnectionPool min and max sizes TINK...

2018-09-11 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/903 Thanks for the insight @spmallette. That means for this PR that we don't have to hurry at the moment. > Max in-flight should be a threshold that, when reached, more connecti

[GitHub] tinkerpop issue #903: Gremlin.Net: Add ConnectionPool min and max sizes TINK...

2018-09-11 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/903 > I think implementing TINKERPOP-1774 with the current implementation of the pool (dequeuing) comes at a cost of introducing complexity (in particular AsyncAutoResetEvent).

[GitHub] tinkerpop issue #903: Gremlin.Net: Add ConnectionPool min and max sizes TINK...

2018-09-10 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/903 Ok, I think I finally got your proposition. Took me way longer than it should have to be honest 😅 The changes would basically look like this if I got everything right

[GitHub] tinkerpop issue #903: Gremlin.Net: Add ConnectionPool min and max sizes TINK...

2018-09-10 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/903 >>What I meant here is that the connection cannot be used again until SendAsync completed. So, my suggestion for request pipelining is simply that the connection is taken out of th

[GitHub] tinkerpop issue #903: Gremlin.Net: Add ConnectionPool min and max sizes TINK...

2018-09-10 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/903 >>Why do you want to keep the connections in the pool when a read/write operation is executed on them? They can't be used at that time anyway and I think that the current design is

[GitHub] tinkerpop issue #915: Tinkerpop 1913-Followup

2018-09-07 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/915 > Is there a good reason for not exposing the StatusCode on the ResponseException as well? I'm not familiar enough with the protocol to say whether that's an internal detail or someth

[GitHub] tinkerpop issue #903: Gremlin.Net: Add ConnectionPool min and max sizes TINK...

2018-09-07 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/903 >I think the ultimate solution would be for the pool to balance request between connections, and without taking them out of the pool each time a request/response is issued.

[GitHub] tinkerpop issue #903: Gremlin.Net: Add ConnectionPool min and max sizes TINK...

2018-09-06 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/903 Thanks for the clarification. I think I understand now how that would look like. It is of course a nice improvement. However, I wonder what would be the concrete advantages for the user

[GitHub] tinkerpop issue #926: TINKERPOP-2031 Removed support for -i in gremlin-serve...

2018-09-06 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/926 LGTM, VOTE +1 ---

[GitHub] tinkerpop pull request #925: Make closing of connections robust TINKERPOP-20...

2018-09-01 Thread FlorianHockmann
GitHub user FlorianHockmann opened a pull request: https://github.com/apache/tinkerpop/pull/925 Make closing of connections robust TINKERPOP-2026 https://issues.apache.org/jira/browse/TINKERPOP-2026 This should solve the problem described in the ticket where apparently

[GitHub] tinkerpop pull request #903: Gremlin.Net: Add ConnectionPool min and max siz...

2018-08-31 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/903#discussion_r214426405 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/AsyncAutoResetEvent.cs --- @@ -0,0 +1,103 @@ +#region License

[GitHub] tinkerpop issue #903: Gremlin.Net: Add ConnectionPool min and max sizes TINK...

2018-08-31 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/903 > We should allow sending multiple requests without waiting for a response and support unordered responses (based on the request id) Wouldn't that mean that the server se

[GitHub] tinkerpop pull request #903: Gremlin.Net: Add ConnectionPool min and max siz...

2018-08-31 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/903#discussion_r214384104 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/AsyncAutoResetEvent.cs --- @@ -0,0 +1,103 @@ +#region License

[GitHub] tinkerpop pull request #903: Gremlin.Net: Add ConnectionPool min and max siz...

2018-08-31 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/903#discussion_r214373377 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs --- @@ -33,91 +34,135 @@ internal class ConnectionPool : IDisposable

[GitHub] tinkerpop pull request #903: Gremlin.Net: Add ConnectionPool min and max siz...

2018-08-31 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/903#discussion_r214371239 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/AsyncAutoResetEvent.cs --- @@ -0,0 +1,103 @@ +#region License

[GitHub] tinkerpop pull request #903: Gremlin.Net: Add ConnectionPool min and max siz...

2018-08-30 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/903#discussion_r214055785 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs --- @@ -33,91 +34,134 @@ internal class ConnectionPool : IDisposable

[GitHub] tinkerpop issue #912: TINKERPOP-2023 SSL Enhancements

2018-08-20 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/912 > They settings are @Deprecated not removed - for example: Ah, thanks for the clarification. I indeed only looked at the changes to the docs. Deprecating those setti

[GitHub] tinkerpop issue #912: TINKERPOP-2023 SSL Enhancements

2018-08-20 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/912 To be honest, I haven't taken a detailed look at the changes of this PR yet, but it seems to also remove / rename a bunch of config properties like `ssl.keyFile` for example. Doesn't

[GitHub] tinkerpop issue #915: Tinkerpop 1913-Followup

2018-08-18 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/915 In that case, you can change it with: ``` git config --global user.name "John Doe" ``` and then reset your two commits so you can write a new commit message and

[GitHub] tinkerpop issue #912: TINKERPOP-2023 SSL Enhancements

2018-08-18 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/912 Since this is a breaking change, wouldn't it be better to target `master` so that this can go into 3.4.0? ---

[GitHub] tinkerpop pull request #903: Gremlin.Net: Add ConnectionPool min and max siz...

2018-08-17 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/903#discussion_r210924567 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs --- @@ -33,91 +34,134 @@ internal class ConnectionPool : IDisposable

[GitHub] tinkerpop pull request #915: Tinkerpop 1913-Followup

2018-08-17 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/915#discussion_r210893971 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/IResultSet.cs --- @@ -0,0 +1,43 @@ +#region License +/* + * Licensed to the Apache

[GitHub] tinkerpop pull request #915: Tinkerpop 1913-Followup

2018-08-17 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/915#discussion_r210903830 --- Diff: gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Driver/GremlinClientTests.cs --- @@ -51,7 +52,18 @@ public async Task

[GitHub] tinkerpop pull request #915: Tinkerpop 1913-Followup

2018-08-17 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/915#discussion_r210902761 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ResultSet.cs --- @@ -0,0 +1,85 @@ +#region License +/* + * Licensed to the Apache

[GitHub] tinkerpop pull request #915: Tinkerpop 1913-Followup

2018-08-17 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/915#discussion_r210902499 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ResultSet.cs --- @@ -0,0 +1,85 @@ +#region License +/* + * Licensed to the Apache

[GitHub] tinkerpop pull request #915: Tinkerpop 1913-Followup

2018-08-17 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/915#discussion_r210901000 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs --- @@ -116,9 +118,9 @@ private async Task> ReceiveAs

[GitHub] tinkerpop pull request #915: Tinkerpop 1913-Followup

2018-08-17 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/915#discussion_r210901961 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ResultSet.cs --- @@ -0,0 +1,85 @@ +#region License +/* + * Licensed to the Apache

[GitHub] tinkerpop pull request #915: Tinkerpop 1913-Followup

2018-08-17 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/915#discussion_r210901733 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ResultSet.cs --- @@ -0,0 +1,85 @@ +#region License +/* + * Licensed to the Apache

[GitHub] tinkerpop pull request #915: Tinkerpop 1913-Followup

2018-08-17 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/915#discussion_r210895827 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/IResultSet.cs --- @@ -0,0 +1,43 @@ +#region License +/* + * Licensed to the Apache

[GitHub] tinkerpop issue #911: TINKERPOP-2024 Make server archetype use remote traver...

2018-08-13 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/911 Thanks for implementing this so quickly, @spmallette LGTM, VOTE +1 ---

[GitHub] tinkerpop issue #842: TINKERPOP-1945

2018-08-13 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/842 I think I found the cause of the problem: We're using `dynamic` in Gremlin.Net which requires the `Microsoft.CSharp` package. We always got that package because Newtonsoft.Json also had

[GitHub] tinkerpop issue #842: TINKERPOP-1945

2018-08-13 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/842 I'm looking into this. At a first glance it seems to be based on the combination of the upgraded Newtonsoft.Json version and the addition of the .NET Standard 2.0 target. ---

[GitHub] tinkerpop pull request #842: TINKERPOP-1945

2018-08-13 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/842#discussion_r209610435 --- Diff: gremlin-dotnet/src/Gremlin.Net/Gremlin.Net.csproj --- @@ -48,7 +48,7 @@ Please see the reference documentation at Apache TinkerPop

[GitHub] tinkerpop pull request #842: TINKERPOP-1945

2018-08-13 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/842#discussion_r209602159 --- Diff: gremlin-dotnet/src/Gremlin.Net/Gremlin.Net.csproj --- @@ -48,7 +48,7 @@ Please see the reference documentation at Apache TinkerPop

[GitHub] tinkerpop issue #842: TINKERPOP-1945

2018-08-13 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/842 Great, thanks for making those changes. My VOTE +1 from nearly 3 months ago still holds with these recent changes. ---

[GitHub] tinkerpop pull request #842: TINKERPOP-1945

2018-08-10 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/842#discussion_r209342928 --- Diff: gremlin-dotnet/test/Gremlin.Net.UnitTest/Structure/IO/GraphSON/GraphSONWriterTests.cs --- @@ -346,6 +347,70 @@ public void

[GitHub] tinkerpop pull request #842: TINKERPOP-1945

2018-08-10 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/842#discussion_r209346542 --- Diff: gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphSON/TimeSpanDeserializer.cs --- @@ -0,0 +1,37 @@ +#region License

[GitHub] tinkerpop issue #909: TINKERPOP-1976 GLV testing for computer tests

2018-08-08 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/909 Great to now also have the computer tests for the GLVs :tada: VOTE +1 ---

[GitHub] tinkerpop issue #842: TINKERPOP-1945

2018-08-05 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/842 @danielcweber the build is now failing because some of the added files miss the required license header. Could you please add that? ---

[GitHub] tinkerpop issue #904: TINKERPOP-2016 Bumped to Jackson 2.9.6

2018-08-03 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/904 At least they are working on it for a future release: Ticket: https://issues.apache.org/jira/browse/SPARK-24601 PR: https://github.com/apache/spark/pull/21596 Although

[GitHub] tinkerpop issue #904: TINKERPOP-2016 Bumped to Jackson 2.9.6

2018-08-03 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/904 Ah ok, I didn't see that. I guess I'm still not really used to the way Maven works 😅 But in that case: VOTE +1 ---

[GitHub] tinkerpop issue #904: TINKERPOP-2016 Bumped to Jackson 2.9.6

2018-08-03 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/904 I just searched whether the library would also be used somewhere else and found it [in `spark-gremlin`](https://github.com/apache/tinkerpop/blob/tp32/spark-gremlin/pom.xml#L211

[GitHub] tinkerpop issue #903: Gremlin.Net: Add ConnectionPool min and max sizes TINK...

2018-08-03 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/903 I rebased and added a commit that should address your comments. The reference docs now contain a _Configuration_ section for Gremlin.Net. I was also able to run all tests

[GitHub] tinkerpop pull request #903: Gremlin.Net: Add ConnectionPool min and max siz...

2018-08-02 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/903#discussion_r207262352 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPoolSettings.cs --- @@ -0,0 +1,55 @@ +#region License

[GitHub] tinkerpop pull request #903: Gremlin.Net: Add ConnectionPool min and max siz...

2018-08-02 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/903#discussion_r207222032 --- Diff: docs/src/upgrade/release-3.4.x.asciidoc --- @@ -177,6 +177,17 @@ when dealing with that event. To make this easier, the event now

[GitHub] tinkerpop pull request #903: Gremlin.Net: Add ConnectionPool min and max siz...

2018-08-02 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/903#discussion_r207220819 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPoolSettings.cs --- @@ -0,0 +1,55 @@ +#region License

[GitHub] tinkerpop pull request #903: Gremlin.Net: Add ConnectionPool min and max siz...

2018-08-01 Thread FlorianHockmann
GitHub user FlorianHockmann opened a pull request: https://github.com/apache/tinkerpop/pull/903 Gremlin.Net: Add ConnectionPool min and max sizes TINKERPOP-1774 https://issues.apache.org/jira/browse/TINKERPOP-1774 The Gremlin.Net `ConnectionPool` now has min and max sizes

[GitHub] tinkerpop issue #882: TINKERPOP-1990 Add a shortestPath() step

2018-07-30 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/882 Just wondered: Does the _Shortest Path_ recipe still make sense with the new `shortestPath()` step? And should the section in the reference docs about shortest paths and the recipe link

[GitHub] tinkerpop issue #897: TINKERPOP-1967 connectedComponent()

2018-07-28 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/897 Shouldn't we also add the `ConnectedComponent` class to the GLVs so users can easily do something like this in other languages: ```java g.V().hasLabel('person

[GitHub] tinkerpop pull request #895: TINKERPOP-2012 Add .NET Standard 2.0 target to ...

2018-07-22 Thread FlorianHockmann
GitHub user FlorianHockmann opened a pull request: https://github.com/apache/tinkerpop/pull/895 TINKERPOP-2012 Add .NET Standard 2.0 target to Gremlin.Net https://issues.apache.org/jira/browse/TINKERPOP-2012 This simply adds .NET Standard 2.0 as an additional target

[GitHub] tinkerpop issue #894: TINKERPOP-2011 Use NumberHelper on choose()

2018-07-21 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/894 Thanks for handling the `.gitignore`. I also reviewed the code and it looks good to me. So: VOTE +1 ---

[GitHub] tinkerpop issue #894: TINKERPOP-2011 Use NumberHelper on choose()

2018-07-20 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/894 Looks like I accidentally added that `Dockerfile`. I should have checked more carefully what my commit added but shouldn't we prevent that from happening again with an entry

[GitHub] tinkerpop pull request #885: TINKERPOP-1978 Gremlin.Net: Add check for conne...

2018-06-25 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/885#discussion_r197855449 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs --- @@ -45,19 +44,28 @@ public ConnectionPool(ConnectionFactory

[GitHub] tinkerpop pull request #885: Add check for connection state before returning...

2018-06-23 Thread FlorianHockmann
GitHub user FlorianHockmann opened a pull request: https://github.com/apache/tinkerpop/pull/885 Add check for connection state before returning it TINKERPOP-1978 https://issues.apache.org/jira/browse/TINKERPOP-1978 This PR simply adds the requested check of whether

[GitHub] tinkerpop issue #867: TINKERPOP-1836 Add Gremlin.Net.Template project

2018-06-19 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/867 Rebased again to fix a merge conflict. ---

[GitHub] tinkerpop pull request #867: TINKERPOP-1836 Add Gremlin.Net.Template project

2018-06-16 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/867#discussion_r195899631 --- Diff: .travis.yml --- @@ -20,9 +20,13 @@ install: before_install: - wget -q https://packages.microsoft.com/config/ubuntu/14.04

[GitHub] tinkerpop issue #867: TINKERPOP-1836 Add Gremlin.Net.Template project

2018-06-14 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/867 I just rebased and made the changes so that now the template is only packed with: `mvn clean install -Dnuget`. ---

[GitHub] tinkerpop issue #867: TINKERPOP-1836 Add Gremlin.Net.Template project

2018-06-11 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/867 > we require nuget (to run nuget pack) only as part of the deploy phase. I understand that part and I don't have anything against it, especially when we add something to the &

[GitHub] tinkerpop issue #802: Add docker images for console and server TINKERPOP-189...

2018-06-05 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/802 > Please make a ticket in JIRA as a reminder for that and assign it a fix version of 3.4.0 so we don't forget. Done: [TINKERPOP-1980](https://issues.apache.org/jira/bro

[GitHub] tinkerpop issue #802: Add docker images for console and server TINKERPOP-189...

2018-06-04 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/802 Just pushed two CTR commits to hopefully get the tags right: [aac534e11](https://github.com/apache/tinkerpop/commit/aac534e11fa406bedc61dbc060d06740b898e170) adds a check to only

[GitHub] tinkerpop issue #802: Add docker images for console and server TINKERPOP-189...

2018-06-04 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/802 > I believe that aspect of this works fine. Nothing deploys when the version is a SNAPSHOT. But do we want to add the `3.2` tag for a `3.2.10-rc1` image? That means that us

[GitHub] tinkerpop issue #802: Add docker images for console and server TINKERPOP-189...

2018-06-04 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/802 > Separately, do I need to edit the repository short/long description manually? can that not be automatically done on deploy using content from our github repo? I'm afr

[GitHub] tinkerpop issue #802: Add docker images for console and server TINKERPOP-189...

2018-06-04 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/802 > I only see the 3.2 tag - where the 3.2.10-rc1? shouldn't that be up there too? Yes and the `3.2` tag should only exist for non-SNAPSHOT versions, but I guess that we did

[GitHub] tinkerpop issue #802: Add docker images for console and server TINKERPOP-189...

2018-06-04 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/802 > I also just pushed a minor update to make the command more like our others with -DdockerImages rather than explicitly using the -P directly which we typically dont' do. Also disab

[GitHub] tinkerpop issue #802: Add docker images for console and server TINKERPOP-189...

2018-05-30 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/802 @spmallette you now voted +1 on this but didn't you want to push release candidates of the Docker images during the review? ---

[GitHub] tinkerpop pull request #802: Add docker images for console and server TINKER...

2018-05-30 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/802#discussion_r191811843 --- Diff: docs/src/dev/developer/release.asciidoc --- @@ -111,7 +111,8 @@ under release is protected. Tweaks to documentation and other odds

[GitHub] tinkerpop issue #842: TINKERPOP-1945

2018-05-24 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/842 In case someone is following here but is not on the dev mailing list: https://lists.apache.org/thread.html/32c47e8682c2848a9cc90eaff0a0da1f9bf4bd5a94b746a3e143f341

[GitHub] tinkerpop issue #842: TINKERPOP-1945

2018-05-24 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/842 > @FlorianHockmann Why would we do that? If we can construct (i.e. deserialize) e.g. a DateTime of whatever a server gives us, fine. Doesn't mean we have to send (i.e. serialize) obsc

[GitHub] tinkerpop issue #842: TINKERPOP-1945

2018-05-23 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/842 After thinking a bit more about this, my conclusion was that deprecating those types doesn't really make sense unless we plan on GraphSON 4.0 as we couldn't remove the types before

[GitHub] tinkerpop issue #868: Tinkerpop 1913

2018-05-23 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/868 @ashwini-ms did you see my comment regarding whether `IGremlinClient` should return a `ResultSet` now instead of an `IReadOnlyCollection`? It's now a bit hidden as GitHub collapsed

[GitHub] tinkerpop issue #802: Add docker images for console and server TINKERPOP-189...

2018-05-23 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/802 No, you're right. That is something that is very unlikely to occur and I can't think of other (more likely) reasons where the build of the docker images could fail as they really only

[GitHub] tinkerpop issue #802: Add docker images for console and server TINKERPOP-189...

2018-05-23 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/802 > what is the advantage to building these images if a .docker file is present? A reason to keep this option is that it makes it easier to detect when changes break the Doc

[GitHub] tinkerpop pull request #802: Add docker images for console and server TINKER...

2018-05-23 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/802#discussion_r190320705 --- Diff: docs/src/reference/gremlin-applications.asciidoc --- @@ -353,6 +353,38 @@ variable initialization code into the console. Like

[GitHub] tinkerpop pull request #802: Add docker images for console and server TINKER...

2018-05-23 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/802#discussion_r190319039 --- Diff: gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ResultQueueTest.java --- @@ -281,7 +281,7 @@ public void

[GitHub] tinkerpop issue #842: TINKERPOP-1945

2018-05-22 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/842 I'm ok with either direction - either to only support types symmetrically or try to support at the reading site as much types as possible - as long as we make that decision clear

[GitHub] tinkerpop issue #867: TINKERPOP-1836 Add Gremlin.Net.Template project

2018-05-21 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/867 > so if the version changed from 4..4.1 Good point, I haven't considered that. I amended my last commit to include the solution you proposed. The downloaded exe now sim

[GitHub] tinkerpop issue #867: TINKERPOP-1836 Add Gremlin.Net.Template project

2018-05-21 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/867 You're right. It shouldn't be necessary to have an internet connection when all dependencies are already available. I thought that nuget.exe would only be downloaded once as we have

[GitHub] tinkerpop issue #802: Add docker images for console and server TINKERPOP-189...

2018-05-19 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/802 Rebased and removed the hard-coded version. The major and minor version are now parsed with the `build-helper-maven-plugin` and added as tags to the image when it's not a `SNAPSHOT

[GitHub] tinkerpop issue #867: TINKERPOP-1836 Add Gremlin.Net.Template project

2018-05-18 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/867 > Or, would it be crazy to only package templates if mono is present? I think that would be the best solution here. It wouldn't introduce any new dependencies to the bu

[GitHub] tinkerpop pull request #802: Add docker images for console and server TINKER...

2018-05-18 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/802#discussion_r189340476 --- Diff: gremlin-console/pom.xml --- @@ -336,5 +336,59 @@ limitations under the License

[GitHub] tinkerpop pull request #868: Tinkerpop 1913

2018-05-18 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/868#discussion_r189275849 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ResultSet.cs --- @@ -0,0 +1,88 @@ +#region License + +/* + * Licensed

[GitHub] tinkerpop issue #867: TINKERPOP-1836 Add Gremlin.Net.Template project

2018-05-16 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/867 Rebased and I extended the `gremlin-archetypes` section in the reference docs by splitting it into two sections: _Maven Archetypes_ and _Gremlin.Net Template_. In the future, we can add

[GitHub] tinkerpop issue #842: [WIP] TINKERPOP-1945

2018-05-16 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/842 Great and thanks for your contribution @danielcweber! VOTE +1 ---

[GitHub] tinkerpop pull request #842: [WIP] TINKERPOP-1945

2018-05-16 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/842#discussion_r188710881 --- Diff: gremlin-dotnet/test/Gremlin.Net.UnitTest/Structure/IO/GraphSON/GraphSONReaderTests.cs --- @@ -39,7 +39,13 @@ public class

[GitHub] tinkerpop pull request #842: [WIP] TINKERPOP-1945

2018-05-16 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/842#discussion_r188686243 --- Diff: gremlin-dotnet/test/Gremlin.Net.UnitTest/Structure/IO/GraphSON/GraphSONReaderTests.cs --- @@ -39,7 +39,13 @@ public class

[GitHub] tinkerpop issue #842: [WIP] TINKERPOP-1945

2018-05-16 Thread FlorianHockmann
Github user FlorianHockmann commented on the issue: https://github.com/apache/tinkerpop/pull/842 Yeah, but they all extend `NumberConverter`, don't they? I remember that there was a decision for gremlin-python a while back to separate serializers and deserializers in different

[GitHub] tinkerpop pull request #868: Tinkerpop 1913

2018-05-16 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/868#discussion_r188655675 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ResultSet.cs --- @@ -0,0 +1,88 @@ +#region License + +/* + * Licensed

[GitHub] tinkerpop pull request #868: Tinkerpop 1913

2018-05-16 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/868#discussion_r188657021 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ResultSet.cs --- @@ -0,0 +1,88 @@ +#region License + +/* + * Licensed

[GitHub] tinkerpop pull request #868: Tinkerpop 1913

2018-05-16 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/868#discussion_r188657628 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ResultSet.cs --- @@ -0,0 +1,88 @@ +#region License + +/* + * Licensed

[GitHub] tinkerpop pull request #868: Tinkerpop 1913

2018-05-16 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/868#discussion_r188658381 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ResultSet.cs --- @@ -0,0 +1,88 @@ +#region License + +/* + * Licensed

[GitHub] tinkerpop pull request #868: Tinkerpop 1913

2018-05-16 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/868#discussion_r188654244 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ResultSet.cs --- @@ -0,0 +1,88 @@ +#region License + +/* + * Licensed

[GitHub] tinkerpop pull request #868: Tinkerpop 1913

2018-05-16 Thread FlorianHockmann
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/868#discussion_r188661041 --- Diff: gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Driver/GremlinClientTests.cs --- @@ -167,6 +168,25 @@ public class GremlinClientTests

  1   2   3   >