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

2018-01-12 Thread dkuppitz 
Github user dkuppitz commented on the issue:

https://github.com/apache/tinkerpop/pull/767
  
VOTE: +1


---

[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 issue #767: TINKERPOP-1858: HttpChannelizer Regression: Does not c...

2018-01-11 Thread spmallette 
Github user spmallette commented on the issue:

https://github.com/apache/tinkerpop/pull/767
  
VOTE +1


---

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

2018-01-10 Thread spmallette 
Github user spmallette commented on the issue:

https://github.com/apache/tinkerpop/pull/767
  
I've seen this kind of thing happen before and we usually handle the dual 
exception the way that you have it there by catching either exception or 
catching generically and asserting either one as acceptable.


---

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

2018-01-10 Thread krlohnes 
Github user krlohnes commented on the issue:

https://github.com/apache/tinkerpop/pull/767
  
@robertdale `NoHttpResponseException` was the exception that I was getting 
when I wrote the test and ran it locally (OSX). It seems to be a different 
error in Docker. In Docker I get the `java.net.SocketException`, just like both 
of you got.

I can change it so it only passes in Docker or I can change the test body 
to something like.
```java
try {...}
catch(SocketException | NoHttpResponseException e){}
finally {...}
```

I prefer the `try...catch`, but I will defer to you and @spmallette on that.


---

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

2018-01-09 Thread robertdale 
Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/767
  
Is it just the wrong expected exception in the test?


---

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

2018-01-09 Thread robertdale 
Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/767
  
Failed docker...
```
docker/build.sh -i -t

[ERROR] Tests run: 5, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 
13.448 s <<< FAILURE! - in 
org.apache.tinkerpop.gremlin.server.channel.HttpChannelizerIntegrateTest
[ERROR] 
shouldBreakOnInvalidAuthenticationHandler(org.apache.tinkerpop.gremlin.server.channel.HttpChannelizerIntegrateTest)
  Time elapsed: 2.63 s  <<< ERROR!
java.lang.Exception: Unexpected exception, 
expected but 
was
at 
org.apache.tinkerpop.gremlin.server.channel.HttpChannelizerIntegrateTest.shouldBreakOnInvalidAuthenticationHandler(HttpChannelizerIntegrateTest.java:47)
```


---

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

2018-01-09 Thread spmallette 
Github user spmallette commented on the issue:

https://github.com/apache/tinkerpop/pull/767
  
@dkuppitz or @robertdale could you guys give this a try? maybe it's just me 
for some reason?


---

[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 successful

```
[WARNING] Tests run: 204, Failures: 0, Errors: 0, Skipped: 14
[INFO]
[INFO]
[INFO] --- revapi-maven-plugin:0.8.0:check (default) @ gremlin-server ---
[INFO]
[INFO] --- maven-failsafe-plugin:2.20:verify (verify-integration-test) @ 
gremlin-server ---
[INFO] 

[INFO] BUILD SUCCESS
[INFO] 

[INFO] Total time: 17:58 min
[INFO] Finished at: 2018-01-09T12:05:54-05:00
[INFO] Final Memory: 56M/681M
[INFO] 

```

Running on OSX and jdk 1.8u152 


---

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

2018-01-09 Thread spmallette 
Github user spmallette commented on the issue:

https://github.com/apache/tinkerpop/pull/767
  
when i do:

```text
$ mvn clean install && mvn verify -pl gremlin-server 
-DskipIntegrationTests=false
```

i get this failure consistently:

```text
[ERROR] Errors: 
[ERROR]   
HttpChannelizerIntegrateTest.shouldBreakOnInvalidAuthenticationHandler »  
Unex...
[INFO] 
[ERROR] Tests run: 204, Failures: 0, Errors: 1, Skipped: 14
[INFO] 
[INFO] 
[INFO] --- revapi-maven-plugin:0.8.0:check (default) @ gremlin-server ---
[INFO] 
[INFO] --- maven-failsafe-plugin:2.20:verify (verify-integration-test) @ 
gremlin-server ---
[INFO] 

[INFO] BUILD FAILURE
[INFO] 

[INFO] Total time: 16:52 min
[INFO] Finished at: 2018-01-09T11:09:03-05:00
[INFO] Final Memory: 49M/725M
[INFO] 

```

from the test output it says:

```text

---
Test set: 
org.apache.tinkerpop.gremlin.server.channel.HttpChannelizerIntegrateTest

---
Tests run: 5, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 12.773 s 
<<< FAILURE! - in 
org.apache.tinkerpop.gremlin.server.channel.HttpChannelizerIntegrateTest

shouldBreakOnInvalidAuthenticationHandler(org.apache.tinkerpop.gremlin.server.channel.HttpChannelizerIntegrateTest)
  Time elapsed: 2.46 s  <<< ERROR!
java.lang.Exception: Unexpected exception, 
expected but 
was
at 
org.apache.tinkerpop.gremlin.server.channel.HttpChannelizerIntegrateTest.shouldBreakOnInvalidAuthenticationHandler(HttpChannelizerIntegrateTest.java:47)
```



---

[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] 

[INFO] BUILD SUCCESS
[INFO] 

[INFO] Total time: 18:16 min
[INFO] Finished at: 2018-01-08T14:50:13-05:00
[INFO] Final Memory: 40M/428M
[INFO] 

```


---

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

2018-01-08 Thread spmallette 
Github user spmallette commented on the issue:

https://github.com/apache/tinkerpop/pull/767
  
I'm getting test failures when running Gremlin Server integration tests. 
@krlohnes is that a problem for you?


---

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

2018-01-04 Thread spmallette 
Github user spmallette commented on the issue:

https://github.com/apache/tinkerpop/pull/767
  
Ok - thanks for your thoughts. I can't say we have a release date for 3.3.2 
in mind yet. I'd say it was a couple of months away at least based on how we 
typically do releases. 


---

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

2018-01-04 Thread robertdale 
Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/767
  
VOTE +1


---

[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 right now, but it breaks anyone using 
custom http authentication (e.g. the JanusGraph issue linked to this), so 
that's pretty bad. That, however, only impacts JanusGraph master right now that 
I know of though, so it's not impacting an official release there at the 
moment. So not too bad if there's a patch version planned FSVO soon. I wish I'd 
caught it before the last one that just went out.


---

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

2018-01-04 Thread spmallette 
Github user spmallette commented on the issue:

https://github.com/apache/tinkerpop/pull/767
  
a bad merge - thanks. how bad/critical do you rate this problem? 


---

[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-822877f6fd92f5cd4b57fa2167f92928R74)
 it has `new HttpBasicAuthenticationHandler()` whereas the [previous 
commit](https://github.com/apache/tinkerpop/commit/eae4101c5fb7b8b976c5898a12e180ee23e50269#diff-822877f6fd92f5cd4b57fa2167f92928R74)
 uses `instantiateAuthenticationHandler(settings.authentication)`


---

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

2018-01-04 Thread spmallette 
Github user spmallette commented on the issue:

https://github.com/apache/tinkerpop/pull/767
  
Hi @krlohnes - thanks for this. Did you happen to research the point where 
this regression occurred? 


---