[GitHub] tinkerpop issue #838: TINKERPOP-1822: Change default RepeatStep to DFS

2018-08-11 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/838 @spmallette Yeah, I'm still giving this some thought. Just rebased. ---

[GitHub] tinkerpop pull request #838: TINKERPOP-1822: Change default RepeatStep to DF...

2018-07-11 Thread krlohnes
Github user krlohnes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/838#discussion_r201906049 --- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java --- @@ -273,11 +300,40 @@ public

[GitHub] tinkerpop pull request #838: TINKERPOP-1822: Change default RepeatStep to DF...

2018-07-11 Thread krlohnes
Github user krlohnes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/838#discussion_r201900734 --- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java --- @@ -273,11 +300,40 @@ public

[GitHub] tinkerpop pull request #838: TINKERPOP-1822: Change default RepeatStep to DF...

2018-06-14 Thread krlohnes
Github user krlohnes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/838#discussion_r195496700 --- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java --- @@ -273,11 +300,40 @@ public

[GitHub] tinkerpop pull request #838: TINKERPOP-1822: Change default RepeatStep to DF...

2018-06-04 Thread krlohnes
Github user krlohnes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/838#discussion_r192728027 --- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java --- @@ -273,11 +300,40 @@ public

[GitHub] tinkerpop pull request #838: TINKERPOP-1822: Change default RepeatStep to DF...

2018-05-29 Thread krlohnes
Github user krlohnes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/838#discussion_r191572290 --- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java --- @@ -273,11 +300,40 @@ public

[GitHub] tinkerpop issue #838: TINKERPOP-1822: Add Depth First Search repeat step opt...

2018-04-17 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/838 Thanks @spmallette I'll hold off on any more changes until things are worked out with that DISCUSS thread ---

[GitHub] tinkerpop issue #838: TINKERPOP-1822: Add Depth First Search repeat step opt...

2018-04-15 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/838 @mpollmeier I was running `mvn clean install` locally, and had the same issue with that test stalling out. A few builds before this had different errors in Travis, not just stalling out, so

[GitHub] tinkerpop pull request #838: TINKERPOP-1822: Add Depth First Search repeat s...

2018-04-13 Thread krlohnes
Github user krlohnes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/838#discussion_r181511880 --- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java --- @@ -273,11 +314,37 @@ public

[GitHub] tinkerpop issue #838: TINKERPOP-1822: Add Depth First Search repeat step opt...

2018-04-13 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/838 I'm definitely still missing something around serialization. All of the traversals that are failing in the gryo side of things work in the console. I'm fairly unfamiliar with that part

[GitHub] tinkerpop issue #838: TINKERPOP-1822: Add Depth First Search repeat step opt...

2018-04-13 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/838 I figured out the errors that I was getting with serialization. I think I just missed registering `SearchAlgo` in a spots. I have some legitimate test failures now I think, which makes fixing

[GitHub] tinkerpop pull request #838: TINKERPOP-1822: Add Depth First Search repeat s...

2018-04-13 Thread krlohnes
Github user krlohnes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/838#discussion_r181402263 --- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java --- @@ -273,11 +314,37 @@ public

[GitHub] tinkerpop pull request #838: TINKERPOP-1822: Add Depth First Search repeat s...

2018-04-12 Thread krlohnes
GitHub user krlohnes opened a pull request: https://github.com/apache/tinkerpop/pull/838 TINKERPOP-1822: Add Depth First Search repeat step option You can merge this pull request into a Git repository by running: $ git pull https://github.com/krlohnes/tinkerpop

[GitHub] tinkerpop issue #715: TINKERPOP-1822: change behaviour of repeat step to be ...

2018-02-23 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/715 Last comment for tonight, it looks like that while loop works with TinkerGraph and a simple query. I believe the more complex query I wrote for the application I'm working on may have an issue

[GitHub] tinkerpop issue #715: TINKERPOP-1822: change behaviour of repeat step to be ...

2018-02-23 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/715 So it's not quite fixed. I changed the while loop to ``` int i = 0; while (this.starts.hasNext()) { if (i==0

[GitHub] tinkerpop issue #715: TINKERPOP-1822: change behaviour of repeat step to be ...

2018-02-21 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/715 @mpollmeier Sure thing! One thing that I've noticed is that currently, `emit` doesn't emit in the order I'd expect for multiple children. e.g. for a simple k-ary tree with 4 nodes, though

[GitHub] tinkerpop pull request #715: TINKERPOP-1822: change behaviour of repeat step...

2018-02-19 Thread krlohnes
Github user krlohnes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/715#discussion_r169194905 --- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java --- @@ -271,7 +274,17 @@ public

[GitHub] tinkerpop issue #767: TINKERPOP-1858: HttpChannelizer Regression: Does not c...

2018-01-11 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/767 I added in the CHANGELOG entry ---

[GitHub] tinkerpop pull request #767: TINKERPOP-1858: HttpChannelizer Regression: Doe...

2018-01-11 Thread krlohnes
Github user krlohnes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/767#discussion_r160963168 --- Diff: gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/channel/HttpChannelizerIntegrateTest.java --- @@ -18,15 +18,40

[GitHub] tinkerpop issue #767: TINKERPOP-1858: HttpChannelizer Regression: Does not c...

2018-01-09 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/767 @spmallette I rebased on top of upstream/master and ran `mvn clean install && mvn verify -pl gremlin-server -DskipIntegrationTests=false` And it was su

[GitHub] tinkerpop issue #767: TINKERPOP-1858: HttpChannelizer Regression: Does not c...

2018-01-09 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/767 @spmallette I'm not seeing any integration test failures. ``` [INFO] [INFO] --- maven-failsafe-plugin:2.20:verify (verify-integration-test) @ gremlin-server --- [INFO

[GitHub] tinkerpop issue #767: TINKERPOP-1858: HttpChannelizer Regression: Does not c...

2018-01-04 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/767 @spmallette As far as criticality, that's fairly dependent on the usage of custom authentication handlers in the wild. It's not impacting my current work or any of the work I did at IBM Compose

[GitHub] tinkerpop issue #767: TINKERPOP-1858: HttpChannelizer Regression: Does not c...

2018-01-04 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/767 @spmallette Yeah, it looks like it happened with the `Merge branch TP32` commit. If you look [here](https://github.com/apache/tinkerpop/commit/960fdc11399590280522189b08727e90cd9b629a#diff

[GitHub] tinkerpop pull request #767: TINKERPOP-1858: HttpChannelizer Regression: Doe...

2017-12-29 Thread krlohnes
GitHub user krlohnes opened a pull request: https://github.com/apache/tinkerpop/pull/767 TINKERPOP-1858: HttpChannelizer Regression: Does not create specified AuthenticationHandler You can merge this pull request into a Git repository by running: $ git pull https

[GitHub] tinkerpop pull request #671: Add combined handler master

2017-07-12 Thread krlohnes
GitHub user krlohnes opened a pull request: https://github.com/apache/tinkerpop/pull/671 Add combined handler master @spmallette I think this is pretty much the cleanest I get history with the fixes etc needed for master. You can merge this pull request into a Git repository

[GitHub] tinkerpop issue #618: TINKERPOP-915 Add combined handler for Http and Websoc...

2017-07-11 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/618 @spmallette From the other PR on master I'd had open >Also, your PR looks a little odd - the commit id is not the same as the one on your other PR. I'm not quite sure how to

[GitHub] tinkerpop pull request #663: [TINKERPOP-915]Add combined handler for Http an...

2017-07-10 Thread krlohnes
GitHub user krlohnes opened a pull request: https://github.com/apache/tinkerpop/pull/663 [TINKERPOP-915]Add combined handler for Http and Websockets [TINKERPOP-915](https://issues.apache.org/jira/browse/TINKERPOP-915) Most of this is tests. I added an integration test

[GitHub] tinkerpop issue #618: TINKERPOP-915 Add combined handler for Http and Websoc...

2017-07-10 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/618 @spmallette done. --- 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

[GitHub] tinkerpop issue #618: TINKERPOP-915 Add combined handler for Http and Websoc...

2017-07-06 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/618 @spmallette I hadn't seen that failure before, but that was from a pretty recent change, ya? I don't think I was up to date with `upstream/tp32`. At any rate, I've rebased the branch on my fork

[GitHub] tinkerpop issue #618: TINKERPOP-915 Add combined handler for Http and Websoc...

2017-06-30 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/618 Okay. I got the NIOChannelizer tests passing now as well, things are `final`ed where appropriate, and I've addressed your PR comments. --- If your project is set up for it, you can reply

[GitHub] tinkerpop issue #618: TINKERPOP-915 Add combined handler for Http and Websoc...

2017-06-30 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/618 @spmallette I fixed the review comments and I think I hit all the places I missed the `final` variables. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] tinkerpop pull request #618: TINKERPOP-915 Add combined handler for Http and...

2017-06-30 Thread krlohnes
Github user krlohnes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/618#discussion_r125045753 --- Diff: docs/src/upgrade/release-3.2.x-incubating.asciidoc --- @@ -39,6 +39,13 @@ it has not been promoted as the primary way to add `IoRegistry

[GitHub] tinkerpop pull request #618: TINKERPOP-915 Add combined handler for Http and...

2017-06-30 Thread krlohnes
Github user krlohnes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/618#discussion_r125040344 --- Diff: gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java --- @@ -212,11 +208,6 @@ public Settings

[GitHub] tinkerpop pull request #618: TINKERPOP-915 Add combined handler for Http and...

2017-06-30 Thread krlohnes
Github user krlohnes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/618#discussion_r125039061 --- Diff: gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/channel/AbstractGremlinChannelizerIntegrateTest.java --- @@ -0,0 +1,326

[GitHub] tinkerpop issue #618: TINKERPOP-915 Add combined handler for Http and Websoc...

2017-06-27 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/618 @spmallette I did some testing between the http channelizer and the combined one. With Basic Auth, the mean latency with 1 requests for the combined channelizer was 148.3 ms

[GitHub] tinkerpop issue #618: TINKERPOP-915 Add combined handler for Http and Websoc...

2017-06-19 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/618 My change can be summed up by "Setup the WebSocket stuff and insert the `GremlinEndpointHandler` into the `ChannelPipeline` to serve Http requests when necessary"

[GitHub] tinkerpop issue #618: TINKERPOP-915 Add combined handler for Http and Websoc...

2017-06-19 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/618 I can probably figure something out with JUnit `Parameters` for the `GremlinServerIntegrateTest` and `GremlinServerHttpIntegrateTest`. I'll have a go at it. --- If your project is set up

[GitHub] tinkerpop issue #618: TINKERPOP-915 Add combined handler for Http and Websoc...

2017-06-19 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/618 @spmallette Yeah, I'm on the mailing list, I've been following along. No worries. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well

[GitHub] tinkerpop issue #618: [TINKERPOP-915]Add combined handler for Http and Webso...

2017-06-08 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/618 @spmallette Thanks for the heads up. --- 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

[GitHub] tinkerpop pull request #618: [TINKERPOP-915]Add combined handler for Http an...

2017-06-06 Thread krlohnes
GitHub user krlohnes opened a pull request: https://github.com/apache/tinkerpop/pull/618 [TINKERPOP-915]Add combined handler for Http and Websockets [TINKERPOP-915](https://issues.apache.org/jira/browse/TINKERPOP-915) Most of this is tests. I added an integration test

[GitHub] tinkerpop issue #583: TINKERPOP-1657 Provide abstraction to easily allow dif...

2017-04-05 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/583 Sorry about that. Fixed. --- 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

[GitHub] tinkerpop issue #583: TINKERPOP-1657 Provide abstraction to easily allow dif...

2017-04-04 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/583 @spmallette I've updated the docs to include the changes made in this PR. Let me know if I've missed anything / if you'd like me to reword anything. --- If your project is set up for it, you

[GitHub] tinkerpop pull request #583: TINKERPOP-1657 Provide abstraction to easily al...

2017-04-04 Thread krlohnes
Github user krlohnes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/583#discussion_r109659946 --- Diff: gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Settings.java --- @@ -384,9 +384,25 @@ public SerializerSettings

[GitHub] tinkerpop issue #583: TINKERPOP-1657 Provide abstraction to easily allow dif...

2017-04-04 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/583 I got the integration tests passing, I'm hoping to finish up the documentation changes today. ``` [INFO] Reactor Summary: [INFO] [INFO] Apache TinkerPop

[GitHub] tinkerpop pull request #583: TINKERPOP-1657 Provide abstraction to easily al...

2017-03-30 Thread krlohnes
Github user krlohnes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/583#discussion_r109015202 --- Diff: gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Settings.java --- @@ -387,6 +387,18 @@ public SerializerSettings

[GitHub] tinkerpop pull request #583: TINKERPOP-1657 Provide abstraction to easily al...

2017-03-30 Thread krlohnes
Github user krlohnes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/583#discussion_r109015184 --- Diff: gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Settings.java --- @@ -387,6 +387,18 @@ public SerializerSettings

[GitHub] tinkerpop pull request #583: TINKERPOP-1657 Provide abstraction to easily al...

2017-03-30 Thread krlohnes
Github user krlohnes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/583#discussion_r109015220 --- Diff: gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/AuthenticationHandler.java --- @@ -0,0 +1,52

[GitHub] tinkerpop pull request #583: TINKERPOP-1657 Provide abstraction to easily al...

2017-03-29 Thread krlohnes
Github user krlohnes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/583#discussion_r108750598 --- Diff: gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Settings.java --- @@ -387,6 +387,18 @@ public SerializerSettings

[GitHub] tinkerpop pull request #583: TINKERPOP-1657 Provide abstraction to easily al...

2017-03-29 Thread krlohnes
Github user krlohnes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/583#discussion_r108750337 --- Diff: gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Settings.java --- @@ -387,6 +387,18 @@ public SerializerSettings

[GitHub] tinkerpop issue #583: TINKERPOP-1657 Provide abstraction to easily allow dif...

2017-03-27 Thread krlohnes
Github user krlohnes commented on the issue: https://github.com/apache/tinkerpop/pull/583 You make some very good points. > The Channelizer abstraction might be enough of an abstraction. I'd thought about that a bit, but in general, I'd rather not have to rewr

[GitHub] tinkerpop pull request #583: TINKERPOP-1657 Provide abstraction to easily al...

2017-03-24 Thread krlohnes
GitHub user krlohnes opened a pull request: https://github.com/apache/tinkerpop/pull/583 TINKERPOP-1657 Provide abstraction to easily allow different HttpAuth schemes Abstracting over the http authentication allows for easy extensibility for users/implementors to provide