[GitHub] tinkerpop issue #948: Optimizes Map with enum using the EnumMap implementati...

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #945: TINKERPOP-2053 Support for OptionsStrategy

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #944: TINKERPOP-2041 Text Predicates

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #943: TINKERPOP-2044 Configurable traversal to validate host...

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #942: TINKERPOP-2049 Added with(k) overload

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop pull request #946: TINKERPOP-2037 removed groovy-sql

[robertdale]

GitHub user robertdale opened a pull request:

https://github.com/apache/tinkerpop/pull/946

TINKERPOP-2037 removed groovy-sql


Removed unused groovy-sql dependency.

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

`sh docker/build.sh -i -t` BUILD SUCCESS

VOTE +1


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/apache/tinkerpop TINKERPOP-2037

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/tinkerpop/pull/946.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #946


commit 24ca28e08fa8a672f64c93fe95e5ae9ffa06dbee
Author: Robert Dale 
Date:   2018-10-02T16:24:31Z

TINKERPOP-2037 removed groovy-sql




---

[GitHub] tinkerpop pull request #943: TINKERPOP-2044 Configurable traversal to valida...

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/943#discussion_r221858009
  
--- Diff: 
gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Cluster.java 
---
@@ -575,6 +585,8 @@ SslContext createSSLContext() throws Exception {
 private String trustStore = null;
 private String trustStorePassword = null;
 private String keyStoreType = null;
+private String validationRequest = "''";
+private boolean useBytecodeForValidation = false;
--- End diff --

`useBytecodeForValidation` was added but doesn't appear to be used anywhere 
else.


---

[GitHub] tinkerpop pull request #943: TINKERPOP-2044 Configurable traversal to valida...

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/943#discussion_r221859065
  
--- Diff: 
gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Settings.java 
---
@@ -243,6 +243,8 @@ public static Settings from(final Configuration conf) {
 if (connectionPoolConf.containsKey("keepAliveInterval"))
 cpSettings.keepAliveInterval = 
connectionPoolConf.getLong("keepAliveInterval");
 
+if (connectionPoolConf.containsKey("validationRequest"))
+cpSettings.validationRequest = 
connectionPoolConf.getList("validationRequest").stream().map(Object::toString).collect(Collectors.joining(","));
--- End diff --

Why is this `getList()` instead of `getString()`?


---

[GitHub] tinkerpop issue #942: TINKERPOP-2049 Added with(k) overload

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #941: TINKERPOP-2040 Improve flexibility of GroovyTranslator...

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #944: TINKERPOP-2041 Text Predicates

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/944
  
I would like to see `TP` renamed to `TextP`.


---

[GitHub] tinkerpop issue #920: optmizes collection copy with Collections addAll

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop pull request #939: TINKERPOP-2045 removed duplicate non-indy groov...

[robertdale]

GitHub user robertdale opened a pull request:

https://github.com/apache/tinkerpop/pull/939

TINKERPOP-2045 removed duplicate non-indy groovy core dep

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

Exclude non-indy transitive dep on groovy core.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/apache/tinkerpop TINKERPOP-2045

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/tinkerpop/pull/939.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #939


commit 6e320daa57a9ec9acb19a213f6dd2081e70e2e26
Author: Robert Dale 
Date:   2018-09-25T11:40:50Z

TINKERPOP-2045 removed duplicate non-indy groovy core dep




---

[GitHub] tinkerpop issue #934: Apply StringEscapeUtils in GroovyTranslator#convertToS...

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #935: TINKERPOP-2025 Change to SHA-256/512 and drop SHA-1 fo...

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #930: TINKERPOP-2032 bump jython-standalone 2.7.1

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/930
  
Claiming 7-day rule :smile: 


---

[GitHub] tinkerpop issue #932: TINKERPOP-2033 Maintain order in profile() annotations

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #931: TINKERPOP-2029 ConcurrentModificationException for Inl...

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #930: TINKERPOP-2032 bump jython-standalone 2.7.1

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop pull request #932: TINKERPOP-2033 Maintain order in profile() anno...

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/932#discussion_r217736025
  
--- Diff: 
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/gryo/GryoSerializersV3d0.java
 ---
@@ -427,7 +428,12 @@ else if (1 == arguments)
 output.writeString(object.getName());
 output.writeDouble(object.getDuration(TimeUnit.NANOSECONDS) / 
100d);
 kryo.writeObject(output, object.getCounts());
-kryo.writeObject(output, object.getAnnotations());
+
+// annotations is a synchronized LinkedHashMap - get rid of 
the "synch" for serialization as gryo
+// doesn't know how to deserialize that well and LinkedHashMap 
should work with 3.3.x and previous
+final Map annotations = new LinkedHashMap<>();
+object.getAnnotations().forEach(annotations::put);
+kryo.writeObject(output, annotations);
 
 // kryo might have a problem with LinkedHashMap value 
collections. can't recreate it independently but
--- End diff --

Do we not need to worry about this comment?


---

[GitHub] tinkerpop pull request #931: TINKERPOP-2029 ConcurrentModificationException ...

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/931#discussion_r217720970
  
--- Diff: docs/src/upgrade/release-3.4.x.asciidoc ---
@@ -29,6 +29,29 @@ Please see the 
link:https://github.com/apache/tinkerpop/blob/3.4.0/CHANGELOG.asc
 
 === Upgrading for Users
 
+ Changed infix behavior
+
+The infix notation of `and()` and `or()` now supports an arbitrary number 
of traversals and `ConnectiveStrategy` produces a traversal with the correct 
AND and OR semantics. Furthermore,
+previous versions failed to apply 3 or more `and()` steps in an infix 
notation, this is now fixed.
+
+[source,groovy]
+
+gremlin> g.V().has("name","marko").and().has("age", 
lt(30)).or().has("name","josh").and().has("age", gt(30)).and().out("created")
+==>v[1]
+==>v[4]
+
+
+In previous versions the above traversal 
--- End diff --

Seems like something more is needed here.


---

[GitHub] tinkerpop pull request #931: TINKERPOP-2029 ConcurrentModificationException ...

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/931#discussion_r217732616
  
--- Diff: docs/src/upgrade/release-3.4.x.asciidoc ---
@@ -29,6 +29,29 @@ Please see the 
link:https://github.com/apache/tinkerpop/blob/3.4.0/CHANGELOG.asc
 
 === Upgrading for Users
 
+ Changed infix behavior
+
+The infix notation of `and()` and `or()` now supports an arbitrary number 
of traversals and `ConnectiveStrategy` produces a traversal with the correct 
AND and OR semantics. Furthermore,
+previous versions failed to apply 3 or more `and()` steps in an infix 
notation, this is now fixed.
+
+[source,groovy]
+
+gremlin> g.V().has("name","marko").and().has("age", 
lt(30)).or().has("name","josh").and().has("age", gt(30)).and().out("created")
+==>v[1]
+==>v[4]
+
+
+In previous versions the above traversal 
+[source,groovy]
+
+gremlin> 
g.V().repeat(__.in('traverses').repeat(__.in('develops')).emit()).emit().values('name')
+==>stephen
+==>matthias
+==>marko
+
+
+See: 
link:https://issues.apache.org/jira/browse/TINKERPOP-967[TINKERPOP-967]
--- End diff --

There should be a link to this issue as well - 
https://issues.apache.org/jira/browse/TINKERPOP-2029


---

[GitHub] tinkerpop pull request #931: TINKERPOP-2029 ConcurrentModificationException ...

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/931#discussion_r217731843
  
--- Diff: docs/src/upgrade/release-3.4.x.asciidoc ---
@@ -29,6 +29,29 @@ Please see the 
link:https://github.com/apache/tinkerpop/blob/3.4.0/CHANGELOG.asc
 
 === Upgrading for Users
 
+ Changed infix behavior
+
+The infix notation of `and()` and `or()` now supports an arbitrary number 
of traversals and `ConnectiveStrategy` produces a traversal with the correct 
AND and OR semantics. Furthermore,
+previous versions failed to apply 3 or more `and()` steps in an infix 
notation, this is now fixed.
+
--- End diff --

The graph example is good for those familiar with the graph but it's not 
immediately clear what the difference in behavior is. I really like the example 
from the comments. I think it would be good to add here (not necessarily 
verbatim) for clarity. 
```
# BEHAVIOR

Input: a.or.b.and.c.or.d.and.e.or.f.and.g.and.h.or.i

## BEFORE
Output: or(a, or(and(b, c), or(and(d, e), or(and(and(f, g), h), i

## NOW
Output: or(a, and(b, c), and(d, e), and(f, g, h), i)
```



---

[GitHub] tinkerpop issue #930: TINKERPOP-2032 bump jython-standalone 2.7.1

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/930
  
master: `docker/build.sh -i -t -n` BUILD SUCCESS


---

[GitHub] tinkerpop issue #930: TINKERPOP-2032 bump jython-standalone 2.7.1

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/930
  
tp33: `docker/build.sh -i -t -n`  SUCCESS
master: building...


---

[GitHub] tinkerpop pull request #930: TINKERPOP-2032 bump jython-standalone 2.7.1

[robertdale]

GitHub user robertdale opened a pull request:

https://github.com/apache/tinkerpop/pull/930

TINKERPOP-2032 bump jython-standalone 2.7.1

https://snyk.io/vuln/SNYK-JAVA-ORGPYTHON-31451

Overview
org.python:jython-standalone Affected versions of this package are 
vulnerable to Arbitrary Code Execution by sending a serialized function to the 
deserializer, which in turn will execute the code.

References
[CVE](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2016-4000)
[Jython Bug Report](http://bugs.jython.org/issue2454)
[Fix Commit](https://hg.python.org/jython/rev/d06e29d100c0)


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/apache/tinkerpop TINKERPOP-2032

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/tinkerpop/pull/930.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #930


commit f70d108e0e9cace79565c658e6bac5c7e9f045ba
Author: Robert Dale 
Date:   2018-09-11T12:35:33Z

TINKERPOP-2032 bump jython-standalone 2.7.1




---

[GitHub] tinkerpop issue #919: String loop to String builder [tp32]

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/919
  
@dkuppitz Will do.


---

[GitHub] tinkerpop issue #914: Do not format and reparse eval timeout arg when evalua...

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #919: String loop to String builder [tp32]

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/919
  
Nevermind, still just grapes.


---

[GitHub] tinkerpop issue #919: String loop to String builder [tp32]

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/919
  
Hmm.. it happens on tp32. 


---

[GitHub] tinkerpop issue #919: String loop to String builder [tp32]

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/919
  
@otaviojava was that locally?  the travis build failed due to grapes 
download.


---

[GitHub] tinkerpop pull request #919: String loop to String builder [tp32]

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/919#discussion_r211952645
  
--- Diff: 
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalMetrics.java
 ---
@@ -208,14 +208,14 @@ private void handleNestedTraversals(final 
Traversal.Admin traversal, final Mutab
 private void appendMetrics(final Collection 
metrics, final StringBuilder sb, final int indent) {
 // Append each StepMetric's row. indexToLabelMap values are 
ordered by index.
 for (Metrics m : metrics) {
-String rowName = m.getName();
+final StringBuilder metricName = new 
StringBuilder(m.getName());
 
 // Handle indentation
 for (int ii = 0; ii < indent; ii++) {
-rowName = "  " + rowName;
+metricName.insert(0, "  ");
 }
--- End diff --

Right.


---

[GitHub] tinkerpop pull request #919: String loop to String builder [master]

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/919#discussion_r211948320
  
--- Diff: 
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalMetrics.java
 ---
@@ -208,14 +208,14 @@ private void handleNestedTraversals(final 
Traversal.Admin traversal, final Mutab
 private void appendMetrics(final Collection 
metrics, final StringBuilder sb, final int indent) {
 // Append each StepMetric's row. indexToLabelMap values are 
ordered by index.
 for (Metrics m : metrics) {
-String rowName = m.getName();
+final StringBuilder metricName = new 
StringBuilder(m.getName());
 
 // Handle indentation
 for (int ii = 0; ii < indent; ii++) {
-rowName = "  " + rowName;
+metricName.insert(0, "  ");
 }
--- End diff --

Yes, I get that. What I'm suggesting is that instead of having to `insert` 
data later on thus incurring the cost of array copies, why not put the 
indentation loop first, then everything after can be `append` only.


---

[GitHub] tinkerpop pull request #919: String loop to String builder

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/919#discussion_r211934275
  
--- Diff: 
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalMetrics.java
 ---
@@ -286,10 +286,10 @@ private void appendMetrics(final Collection metrics, final St
 private static String padLeft(final String text, final int 
amountToPad) {
 // not sure why this method needed to exist. stupid string format 
stuff and commons utilities wouldn't
 // work for some reason in the context this method was used above.
-String newText = text;
+final StringBuilder newText = new StringBuilder(text);
 for (int ix = 0; ix < amountToPad; ix++) {
-newText = " " + newText;
+newText.insert(0, " ");
 }
--- End diff --

Would it make sense to put this block before `text` then it won't have to 
do inserts?


---

[GitHub] tinkerpop pull request #919: String loop to String builder

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/919#discussion_r211934120
  
--- Diff: 
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalMetrics.java
 ---
@@ -208,14 +208,14 @@ private void handleNestedTraversals(final 
Traversal.Admin traversal, final Mutab
 private void appendMetrics(final Collection 
metrics, final StringBuilder sb, final int indent) {
 // Append each StepMetric's row. indexToLabelMap values are 
ordered by index.
 for (Metrics m : metrics) {
-String rowName = m.getName();
+final StringBuilder metricName = new 
StringBuilder(m.getName());
 
 // Handle indentation
 for (int ii = 0; ii < indent; ii++) {
-rowName = "  " + rowName;
+metricName.insert(0, "  ");
 }
--- End diff --

Would it make sense to put this block before the name then it won't have to 
do inserts?


---

[GitHub] tinkerpop issue #919: String loop to String builder

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/919
  
@otaviojava did you decompile the bytecode to make sure the compiler didn't 
do that automagically?   Otherwise, StringBuilder vars should be final.


---

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

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/912
  
@k4rthikr That's a little deeper than I want to get into. `sslCipherSuites` 
provides a whitelist. Whitelists are always preferred over blacklists. If this 
one were blacklisted, there are probably 10 others that were missed. I also 
don't want to imply that these are managed and updated by us. There are just 
too many and changes more often than say TLS protocol version. IMO, the scope 
of this work was to set reasonable defaults that made Gremlin Server+Client 
more secure and raise awareness to the user. It will be up to the user to use 
good ciphers, good key lengths, etc.


---

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

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/912
  
Right, when it's security-related, you're given a pass to break anything! 
:rage1: :rage2:  :rage3: :rage4: 

Seriously, the breaking change is only if you relied on the server 
generating self-signed certs or if the client accepted all certs. In the server 
case, you have to configure SSL correctly.  In the client case, you can revert 
to previous behavior with a one line change.  In both cases, I think they are 
trivial enough configuration changes that warrant going back to tp32 to set 
things right.


---

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

[robertdale]

GitHub user robertdale opened a pull request:

https://github.com/apache/tinkerpop/pull/912

TINKERPOP-2023 SSL Enhancements

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

Gremlin Server no longer supports automatically creating self-signed 
certificates.

Cluster client no longer trusts all certs by default as this is an insecure 
configuration. (TINKERPOP-2022)
If no trust store is configured, Cluster will use the default CA certs.
To revert to the previous behavior and accept all certs, it must be 
explicitly configured.

Introduces JKS and PKCS12 support. JKS is the legacy Java Key Store. PKCS12 
has better cross-platform support and is gaining in adoption. Be aware that JKS 
is the default on Java 8.  Java 9 and higher use PKCS12 as the default. Both 
Java keytool and OpenSSL tools can create, read, update PKCS12 files.

Other new features include specifying SSL protocols and cipher suites.

The packaged `*-secure.yaml` files now restrict the protocol to `TLSv1.2` 
by default.

The implication of all of the above changes means that the packaged 
`*-secure.yaml` files no longer "just work". Minimally, the server files must 
be configured with a key/cert.

PEM-based configurations are deprecated, to be removed in a future release.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/apache/tinkerpop TINKERPOP-2023

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/tinkerpop/pull/912.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #912


commit 14f99a46cc939d0dcfe0423a3db4611e90b0eb6f
Author: Robert Dale 
Date:   2018-08-12T01:12:50Z

TINKERPOP-2023 new SSL client, server parameters

commit 317e81280ac1a2c1793d610f3e7fc538a00cab67
Author: Robert Dale 
Date:   2018-08-13T01:50:02Z

TINKERPOP-2023 default to TLSv1.2, updated upgrade notes

commit 48347f235fa5b66268591732fe799ad0d14a4f8b
Author: Robert Dale 
Date:   2018-08-13T02:23:33Z

TINKERPOP-2023 updated docs

commit 2da958b03dda0af828e8bccacac029e64c95c1cc
Author: Robert Dale 
Date:   2018-08-13T19:28:40Z

TINKERPOP-2023 added tests and some fixes




---

[GitHub] tinkerpop issue #907: Fix Sack Split.

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/907
  
@RainMark Could you create an issue in Jira and describe the problem?   See 
also [Contributing code 
changes](https://tinkerpop.apache.org/docs/current/dev/developer/#_contributing_code_changes).
 It would be good to see a test case that fails without the change and passes 
with the change but also satisfies the examples in the documentation.

I also want to make sure you're not running into this
- http://tinkerpop.apache.org/docs/current/reference/#sack-step
```
The first example does not use a split operator and as such, the same map 
is propagated to
all traversers (a global data structure). The second example, demonstrates 
how Map.clone()
ensures that each traverser’s sack contains a unique, local sack.
```



---

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

[robertdale]

GitHub user robertdale opened a pull request:

https://github.com/apache/tinkerpop/pull/904

TINKERPOP-2016 Bumped to Jackson 2.9.6

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

Bumped to Jackson 2.9.6 for CVE.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/apache/tinkerpop TINKERPOP-2016

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/tinkerpop/pull/904.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #904


commit 1829318f6bfce1bc360818c6a0c34492fa69ec9a
Author: Robert Dale 
Date:   2018-08-02T00:02:14Z

TINKERPOP-2016 Bumped to Jackson 2.9.6




---

[GitHub] tinkerpop issue #886: TINKERPOP-1987 Bump to Spark 2.3.1 and Netty 4.1.25.Fi...

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/886
  
@spmallette any idea why the spark test is hanging in travis?


---

[GitHub] tinkerpop issue #876: TINKERPOP-967 Support nested-repeat() structures

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #874: TINKERPOP-1979 Fix math() on OLAP/Spark

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #871: TINKERPOP-1975 Introduced with() step and Configuring ...

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #864: TINKERPOP-1958 Fixed a bug in TinkerGraphCountStrategy

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #860: TINKERPOP-1595 Optimize TraversalVertexProgram

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #861: TINKERPOP-1956 Deprecated Order.incr and Order.decr

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #869: TINKERPOP-1841 Configure python tests in travis

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #862: TINKERPOP-1961 Removed duplication of images in binari...

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/862
  
I walked through the actual release process and this time I do see the 
correct packaging.
VOTE +1


---

[GitHub] tinkerpop issue #862: TINKERPOP-1961 Removed duplication of images in binari...

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/862
  
I did:
1. `bin/process-docs.sh`
2. `mvn install`
3. `find . -name '*.zip'`
4. `unzip -t 
./gremlin-server/target/apache-tinkerpop-gremlin-server-3.2.9-distribution.zip 
| grep images`
I don't see any images
3. `mvn deploy`
I can see the maven-assembly-plugin rebuild the zip file. Still don't see 
any images in any zips



---

[GitHub] tinkerpop issue #862: TINKERPOP-1961 Removed duplication of images in binari...

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/862
  
After running `bin/process-docs.sh` the resulting console/server zip files 
no longer contain `docs/` let alone `images`. Is there a different step that 
needs to be run?


---

[GitHub] tinkerpop issue #844: TINKERPOP-1628 Implement TraversalSelectStep

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/844
  
Finally got docs building again.
VOTE +1


---

[GitHub] tinkerpop issue #855: TINKERPOP-1950 Cached global strategy lookups during t...

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/855
  
Well, you would do use ConcurrentHashMap ;-)
```
static Map<Class, Boolean> LOADED = new ConcurrentHashMap<>();
try {
LOADED.computeIfAbsent(graphOrGraphComputerClass, (unused) -> {

final String graphComputerClassName = null != 
graphOrGraphComputerClass.getDeclaringClass()
? 
graphOrGraphComputerClass.getCanonicalName().replace(
"." + 
graphOrGraphComputerClass.getSimpleName(),
"$" + 
graphOrGraphComputerClass.getSimpleName())
: graphOrGraphComputerClass.getCanonicalName();
try {
Class.forName(graphComputerClassName);
} catch (ClassNotFoundException e) {
throw new RuntimeException(e);
}

// keep track of stuff we already loaded once - stuff in 
this if/statement isn't
// cheap and this
// method gets called a lot, basically every time a new 
traversal gets spun up
// (that includes
// child traversals. perhaps it is possible to just check 
the cache keys for
// this information, but
// it's not clear if this method will be called with 
something not in the cache
// and if it is and
// it results in error, then we'd probably not want to deal 
with this block
// again anyway
return Boolean.TRUE;
});
} catch (Exception e) {
throw new IllegalStateException(e.getCause().getMessage(), 
e.getCause());
}

```



---

[GitHub] tinkerpop issue #857: TINKERPOP-1869 Allowed iterate() after profile()

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #855: TINKERPOP-1950 Cached global strategy lookups during t...

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/855
  
I think `computeIfAbsent()` would have been better :goat: 


---

[GitHub] tinkerpop issue #853: TINKERPOP-1953 Bump to Groovy 2.4.15

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #851: Improve JavaScript Gremlin documentation

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/851
  
@jorgebay don't forget to vote on your own PR! :smile: 


---

[GitHub] tinkerpop issue #839: TINKERPOP-1936 Improved performance of Bytecode deseri...

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #830: TINKERPOP-1143 Moved TraversalEngine to gremlin-test

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #833: TINKERPOP-1903 Migrated Credential DSL to annotation p...

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #832: TINKERPOP-1934 Bumped httpclient to 4.5.5

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #835: TINKERPOP-1705 Removed rebindings API from java driver

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #837: TINKERPOP-1912 Remove MD5 checksums (tp33)

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #836: TINKERPOP-1912 Remove MD5 checksums

[robertdale]

Github user robertdale commented on the issue:

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


---

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

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/838
  
Look at the Serializers in 
`./gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/`


---

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

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/838#discussion_r181408164
  
--- Diff: 
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/branch/RepeatStep.java
 ---
@@ -273,11 +314,37 @@ public RepeatEndStep(final Traversal.Admin traversal) 
{
 super(traversal);
 }
 
+final LinkedList<Traverser.Admin> stashedStarts = new 
LinkedList<>();
+
+private Traverser.Admin nextStart(RepeatStep repeatStep) {
+if (repeatStep.searchAlgo.equals(SearchAlgo.BFS)) {
+return this.starts.next();
+} else {
+if (this.starts.hasNext()) {
+return this.starts.next();
+} else {
+return this.stashedStarts.pop();
+}
+}
+}
+
+@Override
+public boolean hasNext() {
+return super.hasNext() || this.stashedStarts.peek() != null;
--- End diff --

`this.stashedStarts.peek() != null` would be equivalent to 
`!this.stashedStarts.isEmpty()`, no?


---

[GitHub] tinkerpop issue #829: TINKERPOP-1888 Extend max and min to all Comparable pr...

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop pull request #829: TINKERPOP-1888 Extend max and min to all Compar...

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/829#discussion_r179166260
  
--- Diff: docs/src/reference/the-traversal.asciidoc ---
@@ -1515,16 +1516,17 @@ 
link:++http://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/grem
 [[min-step]]
 === Min Step
 
-The `min()`-step (*map*) operates on a stream of numbers and determines 
which is the smallest number in the stream.
+The `min()`-step (*map*) operates on a stream of comparable objects and 
determines which is the smallest object in the stream.
--- End diff --

~~smallest object~~ first object according to its natural sort order


---

[GitHub] tinkerpop pull request #829: TINKERPOP-1888 Extend max and min to all Compar...

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/829#discussion_r179166002
  
--- Diff: docs/src/reference/the-traversal.asciidoc ---
@@ -1471,16 +1471,17 @@ 
link:++http://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/grem
 [[max-step]]
 === Max Step
 
-The `max()`-step (*map*) operates on a stream of numbers and determines 
which is the largest number in the stream.
+The `max()`-step (*map*) operates on a stream of comparable objects and 
determines which is the largest object in the stream.
--- End diff --

~~largest object~~ last object according to its natural sort order


---

[GitHub] tinkerpop issue #825: TINKERPOP-1930 Remove Giraph

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/825
  
Can this be merged?


---

[GitHub] tinkerpop issue #826: Pinned netty to 4.0.x for spark/console

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/826
  
Can this be closed now?


---

[GitHub] tinkerpop issue #826: Pinned netty to 4.0.x for spark/console

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/826
  
Looks like netty is upgraded in Spark 2.3.0 only. 
https://issues.apache.org/jira/browse/SPARK-19552


---

[GitHub] tinkerpop issue #825: TINKERPOP-1930 Remove Giraph

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #621: TINKERPOP-1682 by-modulator optimization strategy

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/621
  
Looks like my line is longer because it has `vertex` instead of `edge`.


---

[GitHub] tinkerpop issue #621: TINKERPOP-1682 by-modulator optimization strategy

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/621
  
@dkuppitz Still, why is it testing `prettyPrint()` and `prettyPrint(160)`?  
What does the `160` do for us?


---

[GitHub] tinkerpop issue #621: TINKERPOP-1682 by-modulator optimization strategy

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/621
  
@dkuppitz   
https://gist.github.com/robertdale/8b5fa41432d29b3e99bf5ff2515dff3d



---

[GitHub] tinkerpop issue #621: TINKERPOP-1682 by-modulator optimization strategy

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/621
  
This test has two `prettyPrint()` assertions.

The first does `prettyPrint()` no args.  It passes. However, it's strange 
in that two of the `found`s catch the same line:
```
IncidentToAdjacentStrategy  [O]   [VertexStep(OUT,vertex), 
GroupStep([VertexStep(IN,vertex), GroupCountStep([VertexStep(BOTH,vertex), 
CountGlobalStep, IsStep(gt(2))])],[FoldStep])
FOUND
FOUND
```

The second one does `prettyPrint(160)`. It has the same duplicate 
strangeness as the first. It is also the one that consistently fails for me. 
The reason being is that the `CountStrategy` line is short a character thus 
doesn't match.
**Test:**
```
if (line.contains("CountStrategy") && line.contains("RangeGlobalStep(0,3)"))
```
**Input:**
```
CountStrategy   [O]   [VertexStep(OUT,vertex), 
GroupStep([VertexStep(IN,vertex), GroupCountStep([VertexStep(BOTH,vertex), 
RangeGlobalStep(0,3
```

I don't know why your `160` chars is different than my `160` chars. But why 
is this being tested anyway?



---

[GitHub] tinkerpop issue #621: TINKERPOP-1682 by-modulator optimization strategy

[robertdale]

Github user robertdale commented on the issue:

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

[ERROR] Tests run: 3, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 
4.187 s <<< FAILURE! - in 
org.apache.tinkerpop.gremlin.process.traversal.util.TraversalExplanationTest
[ERROR] 
shouldApplyStrategiesCorrectly(org.apache.tinkerpop.gremlin.process.traversal.util.TraversalExplanationTest)
  Time elapsed: 0.048 s  <<< FAILURE!
java.lang.AssertionError: expected:<4> but was:<3>
at 
org.apache.tinkerpop.gremlin.process.traversal.util.TraversalExplanationTest.shouldApplyStrategiesCorrectly(TraversalExplanationTest.java:136)
```



---

[GitHub] tinkerpop pull request #621: TINKERPOP-1682 by-modulator optimization strate...

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/621#discussion_r176237542
  
--- Diff: 
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/ByModulatorOptimizationStrategy.java
 ---
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package 
org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization;
+
+import org.apache.tinkerpop.gremlin.process.traversal.Step;
+import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
+import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
+import 
org.apache.tinkerpop.gremlin.process.traversal.lambda.ElementValueTraversal;
+import 
org.apache.tinkerpop.gremlin.process.traversal.lambda.IdentityTraversal;
+import 
org.apache.tinkerpop.gremlin.process.traversal.lambda.TokenTraversal;
+import org.apache.tinkerpop.gremlin.process.traversal.step.ByModulating;
+import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
+import org.apache.tinkerpop.gremlin.process.traversal.step.map.IdStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.map.LabelStep;
+import 
org.apache.tinkerpop.gremlin.process.traversal.step.map.PropertiesStep;
+import 
org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.IdentityStep;
+import 
org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy;
+import org.apache.tinkerpop.gremlin.structure.PropertyType;
+import org.apache.tinkerpop.gremlin.structure.T;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+/**
+ * This strategy looks for standard traversals in by-modulators and 
replaces them with more optimized traversals
+ * (e.g. {@code TokenTraversal}) if possible.
+ * 
+ *
+ * @author Daniel Kuppitz (http://gremlin.guru)
+ * @example 
+ * __.path().by(id())// is replaced by __.path().by(id)
+ * __.dedup().by(label())// is replaced by __.dedup().by(label)
+ * __.group().by(key())  // is replaced by __.group().by(key)
+ * __.group().by(value())// is replaced by __.group().by(value)
--- End diff --

Are key()/value() replaced? Looks like they are commented out below.


---

[GitHub] tinkerpop pull request #621: TINKERPOP-1682 by-modulator optimization strate...

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/621#discussion_r176236647
  
--- Diff: CHANGELOG.asciidoc ---
@@ -110,7 +112,6 @@ This release also includes changes from 
<<release-3-2-6, 3.2.6>>.
 * Removed access to previously deprecated `CoreGremlinPlugin#INSTANCE` 
field.
 * `gremlin.sh` and `gremln.bat` no longer support the option to pass a 
script as an argument for execution mode without using the `-i` option.
 * Graphite and Ganglia are no longer packaged with the Gremlin Server 
distribution.
-* `TransactionException` is no longer a class of `AbstractTransaction` and 
it extends `RuntimeException`.
--- End diff --

Why does this get removed from an earlier version?


---

[GitHub] tinkerpop issue #811: TINKERPOP-1758 Apply RemoteStrategy before all Decorat...

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/811
  
I don't know. Just asking for the record.
VOTE +1


---

[GitHub] tinkerpop issue #811: TINKERPOP-1758 Apply RemoteStrategy before all Decorat...

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/811
  
Does this handle custom strategies?


---

[GitHub] tinkerpop issue #818: Remove null values to fix TINKERPOP-1923

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/818
  
@jbaker-nca  Is there a test case that would fail without this fix so it 
can catch regressions?


---

[GitHub] tinkerpop issue #809: TINKERPOP-1884 Bump to Netty 4.1.21.Final

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/809
  
```
$ docker/build.sh -i -t -n

[INFO] 

[INFO] BUILD SUCCESS
[INFO] 

[INFO] Total time: 02:49 h
[INFO] Finished at: 2018-03-09T22:30:01+00:00
```
VOTE +1



---

[GitHub] tinkerpop issue #808: TINKERPOP-1884 Bump to Netty 4.0.56.Final

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #812: TINKERPOP-1911 Refactored JavaTranslator

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/812
  
What's the difference between the global method cache and the local method 
cache?


---

[GitHub] tinkerpop issue #804: TINKERPOP-1862 Messenger proper handling of Direction....

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #803: TINKERPOP-1522 Order of select() scopes

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #799: TINKERPOP-1586 Added checkAdjacentVertices option to S...

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #793: TINKERPOP-1872 Apply edgeFunction in SparkMessenger

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #800: TINKERPOP-1726 Added idleReadLimit and idleWriteLimit ...

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/800
  
Yes, manually. I was surprised too. In practice, I don't think it's a big 
deal. The only reason I could see even using server-side keep-alive is because 
client-side keep-alive was sufficient. So in that case, you probably wouldn't 
even have client-side keep-alive on.

VOTE +1



---

[GitHub] tinkerpop issue #800: TINKERPOP-1726 Added idleReadLimit and idleWriteLimit ...

[robertdale]

Github user robertdale commented on the issue:

https://github.com/apache/tinkerpop/pull/800
  
The only thing I don't like is that the client keep-alive doesn't seem to 
reset the 'write idle time' so the server still sends ping requests to the 
client even though the client is communicating.


---

[GitHub] tinkerpop pull request #800: TINKERPOP-1726 Added idleReadLimit and idleWrit...

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/800#discussion_r169364908
  
--- Diff: docs/src/reference/gremlin-applications.asciidoc ---
@@ -1073,7 +1073,8 @@ The following table describes the various 
configuration options that Gremlin Ser
 |graphs |A `Map` of `Graph` configuration files where the key of the `Map` 
becomes the name to which the `Graph` will be bound and the value is the file 
name of a `Graph` configuration file. |_none_
 |gremlinPool |The number of "Gremlin" threads available to execute actual 
scripts in a `ScriptEngine`. This pool represents the workers available to 
handle blocking operations in Gremlin Server. When set to `0`, Gremlin Server 
will use the value provided by `Runtime.availableProcessors()`. |0
 |host |The name of the host to bind the server to. |localhost
-|useEpollEventLoop |try to use epoll event loops (works only on Linux os) 
instead of netty NIO. |false
+|idleConnectionTimeout |Time in milliseconds that the server will allow a 
channel to not receive requests from a client before it automatically closes. 
If enabled, the value provided should typically exceed the amount of time given 
to `keepAliveInterval`. Note that while this value is to be provided as 
milliseconds it will resolve to second precision. Set this value to `0` to 
disable this feature. |0
+|keepAliveInterval |Time in milliseconds that the server will allow a 
channel to not send responses to a client before it sends a "ping" to see if it 
is still present. If it is present, the client should respond with a "pong" 
which will thus reset the `#idleConnectionTimeout` and keep the channel open. 
If enabled, this number should be smaller than the value provided to the 
`idleConnectionTimeout`. Note that while this value is to be provided as 
milliseconds it will resolve to second precision. Set this value to `0` to 
disable this feature. |0
--- End diff --

I think you want to remove the `#` on 
```
`#idleConnectionTimeout`
```


---

[GitHub] tinkerpop pull request #800: TINKERPOP-1726 Added idleReadLimit and idleWrit...

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/800#discussion_r169302646
  
--- Diff: CHANGELOG.asciidoc ---
@@ -25,6 +25,7 @@ 
image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 
 * Modified `GremlinDslProcessor` so that it generated the 
`getAnonymousTraversalClass()` method to return the DSL version of `__`.
 * Added the "Kitchen Sink" test data set.
+* Added `idleReadLimit` and `idleWriteLimit` to Gremlin Server that 
enables a "ping" and auto-close for seemingly dead clients.
--- End diff --

I think these should be called something more meaningful.
idleReadLimit:  idleConnectionTimeout
idleWriteLimit: keepAliveInterval


---

[GitHub] tinkerpop pull request #800: TINKERPOP-1726 Added idleReadLimit and idleWrit...

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/800#discussion_r169305818
  
--- Diff: 
gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
 ---
@@ -293,6 +312,21 @@ private static SslContext createServerSslContext() {
 return scriptEngineConf;
 }
 
+@Test
+public void shouldPingChannelIfClientDies() throws Exception {
--- End diff --

You should be able test for a dead client by opening a raw WebSocket, or 
even TCP, connection and simply not responding to anything.

Or separately, just disable the idleWriteLimit (keepalive) and connect. 
Then it should timeout the client.



---

[GitHub] tinkerpop pull request #800: TINKERPOP-1726 Added idleReadLimit and idleWrit...

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/800#discussion_r169302920
  
--- Diff: docs/src/reference/gremlin-applications.asciidoc ---
@@ -1073,7 +1073,8 @@ The following table describes the various 
configuration options that Gremlin Ser
 |graphs |A `Map` of `Graph` configuration files where the key of the `Map` 
becomes the name to which the `Graph` will be bound and the value is the file 
name of a `Graph` configuration file. |_none_
 |gremlinPool |The number of "Gremlin" threads available to execute actual 
scripts in a `ScriptEngine`. This pool represents the workers available to 
handle blocking operations in Gremlin Server. When set to `0`, Gremlin Server 
will use the value provided by `Runtime.availableProcessors()`. |0
 |host |The name of the host to bind the server to. |localhost
-|useEpollEventLoop |try to use epoll event loops (works only on Linux os) 
instead of netty NIO. |false
+|idleReadLimit |Time in milliseconds that the server will allow a channel 
to not receive requests from a client before it automatically closes. If 
enabled, the value provided should typically exceed the amount of time given to 
`idleWriteLimit`. Set this value to `0` to disable this feature. |0
+|idleWriteLimit |Time in milliseconds that the server will allow a channel 
to not send responses to a client before it sends a "ping" to see if it is 
still present. If it is present, the client should respond with a "pong" which 
will thus reset the {@link #idleReadLimit} and keep the channel open. If 
enabled, this number should be smaller than the value provided to the 
`idleReadLimit`. Set this value to `0` to disable this feature. |0
--- End diff --

`{@link #idleReadLimit}` looks like a javadoc comment and doesn't render 
here.


---

[GitHub] tinkerpop pull request #800: TINKERPOP-1726 Added idleReadLimit and idleWrit...

[robertdale]

Github user robertdale commented on a diff in the pull request:

https://github.com/apache/tinkerpop/pull/800#discussion_r169304285
  
--- Diff: 
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/AbstractChannelizer.java
 ---
@@ -147,7 +149,12 @@ public void init(final 
ServerGremlinExecutor serverGremlinExecut
 public void initChannel(final SocketChannel ch) throws Exception {
 final ChannelPipeline pipeline = ch.pipeline();
 
-if (sslContext.isPresent()) pipeline.addLast(PIPELINE_SSL, 
sslContext.get().newHandler(ch.alloc()));
+sslContext.ifPresent(sslContext -> pipeline.addLast(PIPELINE_SSL, 
sslContext.newHandler(ch.alloc(;
+
+// checks for no activity on a channel and triggers an event that 
is consumed by the OpSelectorHandler
+// and either closes the connection or sends a ping to see if the 
client is still alive
+if (supportsIdleMonitor())
+pipeline.addLast(new IdleStateHandler((int) 
(settings.idleReadLimit / 1000), (int) (settings.idleWriteLimit / 1000),0));
--- End diff --

I think we should keep it millis for consistency across configuration 
options.  There could be a note stating it has only second-resolution.


---

[GitHub] tinkerpop issue #795: TINKERPOP-1877 Added the "kitchen sink" toy graph (mas...

[robertdale]

Github user robertdale commented on the issue:

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


---

[GitHub] tinkerpop issue #794: TINKERPOP-1877 Added the "kitchen sink" toy graph

[robertdale]

Github user robertdale commented on the issue:

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


---