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 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 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 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 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 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 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
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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 user krlohnes commented on the issue:
https://github.com/apache/tinkerpop/pull/767
I added in the CHANGELOG entry
---
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 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 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 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 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 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 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 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 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 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 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 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 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 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 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
51 matches
Mail list logo