[GitHub] [tinkerpop]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1029: Updates the API within the core project.
sorry - i've been in the midst of releasing TinkerPop so i haven't had a chance to take a look at this yet. i can probably get to this by the end of the week. [ Full content available at: https://github.com/apache/tinkerpop/pull/1029 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz opened pull request #1030: TINKERPOP-2124 Fixed and/or folding in `InlineFilterStrategy`.
https://issues.apache.org/jira/browse/TINKERPOP-2124 Fixed the bug, added a test case. `docker/build.sh -t -i -n` passed. VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1030 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] aboudreault opened pull request #1031: TINKERPOP-2127: Add Python TraversalMetrics/Metrics deserializers
We only deserialize those types as a dict. Also, this should probably target tp34 when created. [ Full content available at: https://github.com/apache/tinkerpop/pull/1031 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz opened pull request #1032: TINKERPOP-2124 Fixed and/or folding in InlineFilterStrategy (master)
https://issues.apache.org/jira/browse/TINKERPOP-2124 Fixed the bug, added a test case. docker/build.sh -t -i -n passed. VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1032 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz commented on pull request #1029: Updates the API within the core project.
`ThreadLocal.withInitial(ArrayDeque::new)` to follow the previous patterns. [ Full content available at: https://github.com/apache/tinkerpop/pull/1029 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz commented on pull request #1029: Updates the API within the core project.
`ThreadLocal.withInitial(ArrayList::new)` [ Full content available at: https://github.com/apache/tinkerpop/pull/1029 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] otaviojava commented on pull request #1029: Updates the API within the core project.
Thank you, fixed. [ Full content available at: https://github.com/apache/tinkerpop/pull/1029 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] otaviojava commented on pull request #1029: Updates the API within the core project.
Thank you, fixed. [ Full content available at: https://github.com/apache/tinkerpop/pull/1029 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] otaviojava commented on issue #1029: Updates the API within the core project.
retest this please [ Full content available at: https://github.com/apache/tinkerpop/pull/1029 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] FlorianHockmann commented on issue #1029: Updates the API within the core project.
> retest this please I just triggered a new build on Travis. [ Full content available at: https://github.com/apache/tinkerpop/pull/1029 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1029: Updates the API within the core project.
If we have to make these sorts of changes, I think that this PR should target the `tp33` branch so that both of our release branches have the same consistent look to them. [ Full content available at: https://github.com/apache/tinkerpop/pull/1029 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] aboudreault commented on issue #1031: TINKERPOP-2127: Add Python TraversalMetrics/Metrics deserializers
@spmallette to review [ Full content available at: https://github.com/apache/tinkerpop/pull/1031 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] alexliu68 opened pull request #1033: Mask security secret or password
[ Full content available at: https://github.com/apache/tinkerpop/pull/1033 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] alexliu68 opened pull request #1034: Mask security secret or password
[ Full content available at: https://github.com/apache/tinkerpop/pull/1034 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz opened pull request #1035: TINKERPOP-2126 Made TraverserSet internals thread-safe
https://issues.apache.org/jira/browse/TINKERPOP-2126 This PR replaces the `LinkedHashMap` used by `TraverserSet` with a `SynchronizedMap`. I haven't added a test case, cause I really don't know how to consistently provoke a `ConcurrentModificationException`; however, a `ConcurrentModificationException` did happen several times in a provider implementation and it was caused by the `TraverserSet::toString()` method while Spark was a) processing a query and b) concurrently logging the current state. The way `TraverserSet::toString()` is implemented, it totally makes sense that this scenario would sooner or later hit a `ConcurrentModificationException` if we don't use a synchronized map. `docker/build.sh -t -i` passed. VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1035 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] aboudreault commented on issue #1031: TINKERPOP-2127: Add Python TraversalMetrics/Metrics deserializers
This is ready for review. `explain()` doesn't exist in gremlinpython, so g:TraversalExplanation hasn't been added. I've created https://issues.apache.org/jira/browse/TINKERPOP-2128 for the purpose of the missing explain() and check back when it's done. [ Full content available at: https://github.com/apache/tinkerpop/pull/1031 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette opened pull request #1036: Mask security secret or password
https://issues.apache.org/jira/browse/TINKERPOP-2129 NOT READY FOR REVIEW - still testing [ Full content available at: https://github.com/apache/tinkerpop/pull/1036 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1033: Mask security secret or password
closed in favor of #1036 [ Full content available at: https://github.com/apache/tinkerpop/pull/1033 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette closed pull request #1033: Mask security secret or password
[ pull request closed by spmallette ] [ Full content available at: https://github.com/apache/tinkerpop/pull/1033 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1034: Mask security secret or password
closed in favor of #1036 [ Full content available at: https://github.com/apache/tinkerpop/pull/1034 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette closed pull request #1034: Mask security secret or password
[ pull request closed by spmallette ] [ Full content available at: https://github.com/apache/tinkerpop/pull/1034 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] newkek opened pull request #1037: Fix table rendering in Sparql reference docs.
The rendering was off due to the `|` characters in the table. [ Full content available at: https://github.com/apache/tinkerpop/pull/1037 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz commented on issue #1036: Mask security secret or password
VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1036 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] jorgebay commented on issue #1036: Mask security secret or password
VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1036 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] jorgebay commented on issue #1030: TINKERPOP-2124 Fixed and/or folding in InlineFilterStrategy
Nice test, VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1030 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] jorgebay commented on issue #1035: TINKERPOP-2126 Made TraverserSet internals thread-safe
VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1035 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette closed pull request #1036: TINKERPOP-2129 Mask security secret or password
[ pull request closed by spmallette ] [ Full content available at: https://github.com/apache/tinkerpop/pull/1036 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette closed pull request #1037: Fix table rendering in Sparql reference docs.
[ pull request closed by spmallette ] [ Full content available at: https://github.com/apache/tinkerpop/pull/1037 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1037: Fix table rendering in Sparql reference docs.
thanks - we have so many bad formatting errors in the docs on this 3.4.0 release - stinks [ Full content available at: https://github.com/apache/tinkerpop/pull/1037 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1032: TINKERPOP-2124 Fixed and/or folding in InlineFilterStrategy (master)
VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1032 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1035: TINKERPOP-2126 Made TraverserSet internals thread-safe
added entry on the wrong version? should be for 3.3.6 i imagine [ Full content available at: https://github.com/apache/tinkerpop/pull/1035 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1035: TINKERPOP-2126 Made TraverserSet internals thread-safe
Aside from my comment about the CHANAGELOG entry - VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1035 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] nivsherf opened pull request #1038: TINKERPOP-2130 Fix default parameter assignment in Connection ctor
[ Full content available at: https://github.com/apache/tinkerpop/pull/1038 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz commented on pull request #1035: TINKERPOP-2126 Made TraverserSet internals thread-safe
Ha, this time you actually got it right. I was about to answer "no, you get this wrong after every release" :) [ Full content available at: https://github.com/apache/tinkerpop/pull/1035 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] asfgit closed pull request #1035: TINKERPOP-2126 Made TraverserSet internals thread-safe
[ pull request closed by asfgit ] [ Full content available at: https://github.com/apache/tinkerpop/pull/1035 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1035: TINKERPOP-2126 Made TraverserSet internals thread-safe
hahaha - i checked and checked and checked to make sure i wasn't false alarming again! :tada: [ Full content available at: https://github.com/apache/tinkerpop/pull/1035 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1030: TINKERPOP-2124 Fixed and/or folding in InlineFilterStrategy
VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1030 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] jorgebay commented on issue #1032: TINKERPOP-2124 Fixed and/or folding in InlineFilterStrategy (master)
lgtm! VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1032 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz closed pull request #1032: TINKERPOP-2124 Fixed and/or folding in InlineFilterStrategy (master)
[ pull request closed by dkuppitz ] [ Full content available at: https://github.com/apache/tinkerpop/pull/1032 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz closed pull request #1030: TINKERPOP-2124 Fixed and/or folding in InlineFilterStrategy
[ pull request closed by dkuppitz ] [ Full content available at: https://github.com/apache/tinkerpop/pull/1030 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1031: TINKERPOP-2127: Add Python TraversalMetrics/Metrics deserializers
Any reason not to target the `tp33` branch with this? `g:TraversalMetrics` is on the 3.3.x line as well i think. [ Full content available at: https://github.com/apache/tinkerpop/pull/1031 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] aboudreault commented on issue #1031: TINKERPOP-2127: Add Python TraversalMetrics/Metrics deserializers
I think tp33 doesn't have graphson3 so I will need to do another PR [ Full content available at: https://github.com/apache/tinkerpop/pull/1031 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1031: TINKERPOP-2127: Add Python TraversalMetrics/Metrics deserializers
GraphSON 3 was initially released on the 3.3.x line: http://tinkerpop.apache.org/docs/3.3.5/dev/io/#_traversalmetrics_2 [ Full content available at: https://github.com/apache/tinkerpop/pull/1031 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] aboudreault commented on issue #1031: TINKERPOP-2127: Add Python TraversalMetrics/Metrics deserializers
ah. I will update. [ Full content available at: https://github.com/apache/tinkerpop/pull/1031 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] FlorianHockmann opened pull request #1039: TINKERPOP-2131 Gremlin.Net: Better explain connection pool exceptions
https://issues.apache.org/jira/browse/TINKERPOP-2131 Two new exception classes are added to better reveal the reason for a `NoConnectionAvailableException`: * The pool is empty because the server is unavailable -> `ServerUnavailableException` * All connections have reached the max in-flight requests limit -> `ConnectionPoolBusyException` Both extend `NoConnectionAvailableException` so this is a non-breaking change. VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1039 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1038: TINKERPOP-2130 Fix default parameter assignment in Connection ctor
just peeking in on this PR - I'll let @jorgebay make the final word on this, but I guess the reason we needed to add this line was so that calls further down that use `options` and not `this.options` will work properly? shouldn't we just consistently use `this.options` rather than try to interchangeably use both as well as alter the state of the `options` parameter? [ Full content available at: https://github.com/apache/tinkerpop/pull/1038 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1039: TINKERPOP-2131 Gremlin.Net: Better explain connection pool exceptions
VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1039 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1039: TINKERPOP-2131 Gremlin.Net: Better explain connection pool exceptions
VOTE +1 - though a CHANGELOG entry would be good [ Full content available at: https://github.com/apache/tinkerpop/pull/1039 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] jorgebay commented on pull request #1038: TINKERPOP-2130 Fix default parameter assignment in Connection ctor
I think the safest bet would be to use double assignment, that way is concise
and less error prone, dwyt @nivsherf ?
```javascript
this.options = options = options || {};
```
[ Full content available at: https://github.com/apache/tinkerpop/pull/1038 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] FlorianHockmann commented on issue #1039: TINKERPOP-2131 Gremlin.Net: Better explain connection pool exceptions
> though a CHANGELOG entry would be good Right, I just added an entry. [ Full content available at: https://github.com/apache/tinkerpop/pull/1039 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] nivsherf commented on pull request #1038: TINKERPOP-2130 Fix default parameter assignment in Connection ctor
Makes sense @jorgebay. I updated the code accordingly. [ Full content available at: https://github.com/apache/tinkerpop/pull/1038 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] jorgebay commented on issue #1038: TINKERPOP-2130 Fix default parameter assignment in Connection ctor
Thanks for the fix @nivsherf, VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1038 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] aboudreault commented on issue #1031: TINKERPOP-2127: Add Python TraversalMetrics/Metrics deserializers
Rebased for tp33 [ Full content available at: https://github.com/apache/tinkerpop/pull/1031 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz opened pull request #1040: TINKERPOP-1882 Apply range and limit steps as early as possible
https://issues.apache.org/jira/browse/TINKERPOP-1882 `docker/build.sh -t -i -n` passed. VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1040 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz commented on issue #1039: TINKERPOP-2131 Gremlin.Net: Better explain connection pool exceptions
VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1039 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz commented on issue #1038: TINKERPOP-2130 Fix default parameter assignment in Connection ctor
VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1038 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz opened pull request #1041: TINKERPOP-2125 Extend release validation script
https://issues.apache.org/jira/browse/TINKERPOP-2125 Added source file check in release validation script. VOTE +/-0 At this point, the validation does not pass. We need to make a decision on what to do with the following files: ``` ./docs/static/images/gremlin-characters.pdf ./docs/static/images/gremlintron.pdf ./docs/static/images/gremlin-sacks.pdf ./docs/static/images/gremlin-gym.pdf ``` Do we whitelist them or should they be removed from the repository? [ Full content available at: https://github.com/apache/tinkerpop/pull/1041 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] jorgebay closed pull request #1038: TINKERPOP-2130 Fix default parameter assignment in Connection ctor
[ pull request closed by jorgebay ] [ Full content available at: https://github.com/apache/tinkerpop/pull/1038 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] otaviojava commented on issue #1029: Updates the API within the core project.
Nice, as soon it is merged we can work in a cherry-pick :) [ Full content available at: https://github.com/apache/tinkerpop/pull/1029 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1029: Updates the API within the core project.
I hate to be a bother but our git workflow goes from `tp33 --> master`. we typically don't make it a habit of cherrypicking commits backwards in the flow as it makes the git history messy. i'd much prefer a rebase on tp33 with a squash of all these commits into one. we can then merge forward to master. if you need to apply additional changes on master as they only apply there then that can come as a separate PR. In general, it's best to always target tp33 with your PRs unless there is explicit reason not to. If you ever have doubts on what branch to target, feel free to ask ahead of time. sorry for the hassle and thank you. [ Full content available at: https://github.com/apache/tinkerpop/pull/1029 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] skorikov opened pull request #1042: Implemented index lookup for Text predicates
Use index lookup for Text predicates (startingWith,endingWith,containing) in Neo4jGraphStep to improve evaluation performance. See: https://issues.apache.org/jira/browse/TINKERPOP-2133 [ Full content available at: https://github.com/apache/tinkerpop/pull/1042 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1042: TINKERPOP-2133 Implement index lookup for Text predicates
perhaps your IDE did this automatically, but could you please avoid the use of the wildcard import (that's not our code style)? [ Full content available at: https://github.com/apache/tinkerpop/pull/1042 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1042: TINKERPOP-2133 Implement index lookup for Text predicates
thanks for orchestrating this pull request as i know you likely needed changes from neo4j-contrib. i think it's ok to bump to this new version of neo4j for TinkerPop 3.4.1 but since it's a major dependency change I will[ email the dev list](https://lists.apache.org/thread.html/7e96fee99fe6ce2539819f153d5841a670d0abd07fd12084a8fa566a@%3Cdev.tinkerpop.apache.org%3E) to see if there are any objections to doing so. A few issues: 1. There are dependency conflicts being reported during the build by the enforcer plugin. Those will need to be resolved. Note that you might not have noticed them if you built with just `mvn clean install`. We don't automatically run the full tests for neo4j for a number of reasons, so you need to do `mvn clean install -DincludeNeo4j` - in case you're interested you can find a lot of this kind of information in our [developer documentation](http://tinkerpop.apache.org/docs/current/dev/developer/#building-testing). 2. is there any reason you didn't also modify `MultiMetaNeo4jTrait`? 3. are there tests that validate these changes somehow? There are `TextP` tests in the `HastTest` suite of `gremlin-test` but does anything in neo4j need to be configured for the string matching to work? 4. I think this is a big enough change that we need some upgrade documentation. You can add that [here](https://github.com/apache/tinkerpop/blob/master/docs/src/upgrade/release-3.4.x.asciidoc#tinkerpop-341) - that pattern we use should be easy enough to follow to add a new section. [ Full content available at: https://github.com/apache/tinkerpop/pull/1042 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] skorikov commented on pull request #1042: TINKERPOP-2133 Implement index lookup for Text predicates
fixed [ Full content available at: https://github.com/apache/tinkerpop/pull/1042 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1041: TINKERPOP-2125 Extend release validation script
I'm not sure what those are.i assume they are just high-res versions of the png files of the same name? i guess i would just whitelist them since they've been there forever at this point. i assume we can allow *.pdf (we're really trying to mostly avoid compiled code like `.exe`, `.dll`, `.jar`, etc) [ Full content available at: https://github.com/apache/tinkerpop/pull/1041 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] skorikov commented on issue #1042: TINKERPOP-2133 Implement index lookup for Text predicates
@spmallette Thanks! You were right, I did not run `mvn clean install` with `-DincludeNeo4j`; sorry for that. I will address the issues and update the pull request. [ Full content available at: https://github.com/apache/tinkerpop/pull/1042 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz commented on issue #1041: TINKERPOP-2125 Extend release validation script
PDFs are now whitelisted and the validation passes. VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1041 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] robertdale commented on issue #1042: TINKERPOP-2133 Implement index lookup for Text predicates
@skorikov Please include any Neo4j upgrade notes, like we did on [TinkerPop 3.3.1](http://tinkerpop.apache.org/docs/3.4.0/upgrade/#_upgrade_neo4j), in `docs/src/upgrade/release-3.4.x.asciidoc` [ Full content available at: https://github.com/apache/tinkerpop/pull/1042 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] robertdale commented on issue #1042: TINKERPOP-2133 Implement index lookup for Text predicates
@skorikov Please include any Neo4j upgrade notes, like we did on [TinkerPop 3.3.1](http://tinkerpop.apache.org/docs/3.4.0/upgrade/#_upgrade_neo4j), in `docs/src/upgrade/release-3.4.x.asciidoc` Also, ensure that: - all configuration properties in TinkerPop reference are current or note those replaced/updated - test HA configuration [ Full content available at: https://github.com/apache/tinkerpop/pull/1042 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] robertdale commented on issue #1042: TINKERPOP-2133 Implement index lookup for Text predicates
@skorikov I took a quick look at the change and I'm concerned about the choice of using Neo4j version 3.4.0. Let alone that the latest point release for that series is 3.4.11 (12/2018), why not just go to the latest and greatest 3.5.1 (12/2018)? [ Full content available at: https://github.com/apache/tinkerpop/pull/1042 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] porunov opened pull request #1043: Remove redundant code from AbstractStep setId method
This PR removes the redundant `Objects.nonNull(id);` call for `setId` method. [ Full content available at: https://github.com/apache/tinkerpop/pull/1043 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1041: TINKERPOP-2125 Extend release validation script
VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1041 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1040: TINKERPOP-1882 Apply range and limit steps as early as possible
please give me a little extra time to review this one - i had my head in other things last week. [ Full content available at: https://github.com/apache/tinkerpop/pull/1040 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1031: TINKERPOP-2127: Add Python TraversalMetrics/Metrics deserializers
VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1031 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] FlorianHockmann closed pull request #1039: TINKERPOP-2131 Gremlin.Net: Better explain connection pool exceptions
[ pull request closed by FlorianHockmann ] [ Full content available at: https://github.com/apache/tinkerpop/pull/1039 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] FlorianHockmann closed pull request #1026: TINKERPOP-2088 Enable SourceLink for Gremlin.Net
[ pull request closed by FlorianHockmann ] [ Full content available at: https://github.com/apache/tinkerpop/pull/1026 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] jorgebay commented on issue #1031: TINKERPOP-2127: Add Python TraversalMetrics/Metrics deserializers
VOTE +1 :shipit: [ Full content available at: https://github.com/apache/tinkerpop/pull/1031 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1043: Remove redundant code from AbstractStep setId method
Could you please explain this change a bit further? In what way is this null check redundant? where else is a non-null ensured to make this check unnecessary? [ Full content available at: https://github.com/apache/tinkerpop/pull/1043 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1042: TINKERPOP-2133 Implement index lookup for Text predicates
i think @skorikov already had to place some effort into changes in neo4j-contrib to get this to even 3.4.11, so i'm inclined to be content with that version especially since it hasn't been upgraded in a while. I am merely curious though about 3.5.1 since @robertdale mentions it @skorikov was there any reason in particular not to use that latest version? [ Full content available at: https://github.com/apache/tinkerpop/pull/1042 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz commented on issue #1026: TINKERPOP-2088 Enable SourceLink for Gremlin.Net
The Docker builds seem to be broken now. After deleting the existing docker images, I get the following error when `docker.build.sh` tries to rebuild the images: ``` ... Updating certificates in /etc/ssl/certs... 0 added, 0 removed; done. Running hooks in /etc/ca-certificates/update.ddone. Processing triggers for ureadahead (0.100.0-16) ... Errors were encountered while processing: /var/cache/apt/archives/aspnetcore-runtime-2.2_2.2.1-1_amd64.deb E: Sub-process /usr/bin/dpkg returned an error code (1) The command '/bin/sh -c apt-get update && apt-get -y install software-properties-common python-software-properties apt-transport-https curl && echo oracle-java8-installer shared/accepted-oracle-license-v1-1 select true | debconf-set-selections && add-apt-repository -y ppa:webupd8team/java && sh -c 'curl -s https://packages.microsoft.com/config/ubuntu/14.04/packages-microsoft-prod.deb -o packages-microsoft-prod.deb' && sh -c 'dpkg -i packages-microsoft-prod.deb' && sh -c 'echo "deb https://download.mono-project.com/repo/ubuntu stable-trusty main" | sudo tee /etc/apt/sources.list.d/mono-official-stable.list' && apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv-keys 3FA7E0328081BFF6A14DA29AA6A19B38D3D831EF && apt-get update && apt-get install -y oracle-java8-installer gawk git maven openssh-server subversion zip && apt-get install -y --force-yes dotnet-sdk-2.2 python python-dev python3-dev python-pip build-essential mono-devel && pi p install virtualenv virtualenvwrapper && pip install --upgrade pip && rm -rf /var/lib/apt/lists/* /var/cache/oracle-jdk8-installer' returned a non-zero code: 100 ``` [ Full content available at: https://github.com/apache/tinkerpop/pull/1026 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] porunov commented on issue #1043: Remove redundant code from AbstractStep setId method
@spmallette this check returns either `true` of `false`. The returned value isn't used (i.e. the call is redundant). `Objects.nonNull(id);` doesn't throw an exception if the `id` is null. [ Full content available at: https://github.com/apache/tinkerpop/pull/1043 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz commented on issue #1043: Remove redundant code from AbstractStep setId method
Perhaps it should be replaced with `Objects.requireNonNull(...)` then. [ Full content available at: https://github.com/apache/tinkerpop/pull/1043 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] FlorianHockmann commented on issue #1026: TINKERPOP-2088 Enable SourceLink for Gremlin.Net
That's strange. It still works fine for me. I used this to delete any existing images and then start the build: ```bash docker system prune -af docker/build.sh ``` We could in general add another Travis job to run `docker/build.sh` to ensure that we don't accidentally break the Docker build. I remember that we had issues like this already a few times now, although I'm of course not sure whether this approach would have found every problem in the Docker build. [ Full content available at: https://github.com/apache/tinkerpop/pull/1026 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz commented on issue #1026: TINKERPOP-2088 Enable SourceLink for Gremlin.Net
So weird. Failed for me 3 times in a row now. Also, I found this message a bit higher in the log: ``` Selecting previously unselected package dotnet-runtime-2.2. Preparing to unpack .../dotnet-runtime-2.2_2.2.1-1_amd64.deb ... Unpacking dotnet-runtime-2.2 (2.2.1-1) ... dpkg-deb: error: archive '/var/cache/apt/archives/aspnetcore-runtime-2.2_2.2.1-1_amd64.deb' has premature member 'control.tar.xz' before 'control.tar.gz', giving up dpkg: error processing archive /var/cache/apt/archives/aspnetcore-runtime-2.2_2.2.1-1_amd64.deb (--unpack): subprocess dpkg-deb --control returned error exit status 2 ``` [ Full content available at: https://github.com/apache/tinkerpop/pull/1026 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] FlorianHockmann commented on issue #1026: TINKERPOP-2088 Enable SourceLink for Gremlin.Net
``` has premature member 'control.tar.xz' before 'control.tar.gz', giving up ``` Wait, I got exactly the same error in this PR with Travis and added this line to the `.travis.yml` which fixed it there: ``` - sudo apt-get install -y dpkg # workaround for travis-ci/travis-ci#9361 ``` and as the comment indicates, this problem is described here: travis-ci/travis-ci#9361 (where I also got this workaround). Can you try whether it also fixes the problem with the Docker build if you add that line to the Dockerfile? [ Full content available at: https://github.com/apache/tinkerpop/pull/1026 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1043: Remove redundant code from AbstractStep setId method
ah - i was thinking of the exception throwing version when i looked at it. then the @dkuppitz suggestion is probably what we want here. [ Full content available at: https://github.com/apache/tinkerpop/pull/1043 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] porunov commented on issue #1043: Remove redundant code from AbstractStep setId method
@dkuppitz @spmallette Thank you for the clarification. I've replaced it with `Objects.requireNonNull`. [ Full content available at: https://github.com/apache/tinkerpop/pull/1043 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz commented on issue #1026: TINKERPOP-2088 Enable SourceLink for Gremlin.Net
Yep, nice, that solved it! [ Full content available at: https://github.com/apache/tinkerpop/pull/1026 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz commented on issue #1026: TINKERPOP-2088 Enable SourceLink for Gremlin.Net
Well, the images build fine now, but `gremlin-dotnet-source` doesn't. [ Full content available at: https://github.com/apache/tinkerpop/pull/1026 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] FlorianHockmann commented on issue #1026: TINKERPOP-2088 Enable SourceLink for Gremlin.Net
Do you get an exception? [ Full content available at: https://github.com/apache/tinkerpop/pull/1026 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] FlorianHockmann commented on issue #1026: TINKERPOP-2088 Enable SourceLink for Gremlin.Net
Do you get an exception? edit: I created [TINKERPOP-2140](https://issues.apache.org/jira/browse/TINKERPOP-2140) to add the Docker build to our Travis build. [ Full content available at: https://github.com/apache/tinkerpop/pull/1026 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz commented on issue #1026: TINKERPOP-2088 Enable SourceLink for Gremlin.Net
See: https://gist.github.com/dkuppitz/1b912747180b07ab08d4eb98175c119b I'm really wondering how you're able to get a clean build using the same docker images, so confusing... [ Full content available at: https://github.com/apache/tinkerpop/pull/1026 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] FlorianHockmann commented on issue #1026: TINKERPOP-2088 Enable SourceLink for Gremlin.Net
``` [echo] docfx already downloaded. [exec] [19-01-23 03:56:27.186]Info:Config file docfx.json found, start generating metadata... [...] [exec] [19-01-23 03:56:35.566]Error:System.AggregateException: One or more errors occurred. (/projects/apache/tinkerpop/gremlin-dotnet/src/obj/.cache/build/ysbgevf3.uyr/4heij3cn.9cl does not exist) (/projects/apache/tinkerpop/gremlin-dotnet/src/obj/.cache/build/ysbgevf3.uyr/daniuqee.e1y does not exist) (/projects/apache/tinkerpop/gremlin-dotnet/src/obj/.cache/build/ysbgevf3.uyr/0impaqz3.kb3 does not exist) (/projects/apache/tinkerpop/gremlin-dotnet/src/obj/.cache/build/ysbgevf3.uyr/iqzrlx55.yny does not exist) (/projects/apache/tinkerpop/gremlin-dotnet/src/obj/.cache/build/ysbgevf3.uyr/vs6atryj.pzx does not exist) (/projects/apache/tinkerpop/gremlin-dotnet/src/obj/.cache/build/ysbgevf3.uyr/d3rvlip0.vup does not exist) (/projects/apache/tinkerpop/gremlin-dotnet/src/obj/.cache/build/ysbgevf3.uyr/x8o77bsj.6qp does not exist) (/projects/apache/tinkerpop/gremlin-dotnet/src/obj/.cache/build/ysbgevf3.uyr/hqh7xcur.kbb does not exist) ---> System.IO.FileNotFoundException: /projects/apache/tin kerpop/gremlin-dotnet/src/obj/.cache/build/ysbgevf3.uyr/4heij3cn.9cl does not exist ``` That seems to be a problem with DocFX generating the API docs. Can you try it with a cleanly checked out repo (especially without the files generated by DocFX)? [ Full content available at: https://github.com/apache/tinkerpop/pull/1026 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] spmallette commented on issue #1026: TINKERPOP-2088 Enable SourceLink for Gremlin.Net
wonder if that's related to nugetwe still have some odd build problems around clean repos that don't have nuget present i think. i have that on my list of things to review. [ Full content available at: https://github.com/apache/tinkerpop/pull/1026 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz commented on issue #1026: TINKERPOP-2088 Enable SourceLink for Gremlin.Net
It finally worked! I'll add `git -fxd` in our Docker build script. [ Full content available at: https://github.com/apache/tinkerpop/pull/1026 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz commented on issue #1026: TINKERPOP-2088 Enable SourceLink for Gremlin.Net
It finally worked! I'll add `git -fxd` in our Docker build script. EDIT: Dah, no, I won't do that. I tend to run `docker/build.sh -t -i` before I commit my changes (including new files and directories). Thus, this cleanup should be something we should just keep in mind; I don't think we'll need it too often. [ Full content available at: https://github.com/apache/tinkerpop/pull/1026 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] dkuppitz opened pull request #1044: TINKERPOP-2126 Fixed concurrency issue in `ObjectWritable::toString()`.
https://issues.apache.org/jira/browse/TINKERPOP-2126 So, apparently, Spark 2.4.0 has some background logging going on, where it takes objects out of memory and dumps their `toString()` into the log. I don't think we can control when Spark is going to do that, so we have to work around it by making our `toString()` methods thread-safe or handle each `ConcurrentModificationException`. Guaranteeing thread-safety was easy for `TraverserSet` but turned out to be tricky (impossible?) for `ObjectWritable` as it's totally unclear where the objects are coming from and/or where they get modified while Spark is trying to log them. I don't think we can make any arbitrary object thread-safe, but we can easily catch the `ConcurrentModificationException` and retry until we succeed. `while (true)` looks a bit scary, but I don't think that this can ever become an infinite loop. `docker/build.sh -t -i -n` passed (as well as the external provider tests that consistently triggered the concurrency exceptions). VOTE +1 [ Full content available at: https://github.com/apache/tinkerpop/pull/1044 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] skorikov commented on issue #1042: TINKERPOP-2133 Implement index lookup for Text predicates
i guess it is somewhat complicated. neo4j-contrib is not very active, currently, as fas as I know there is only one developer contributing to and maintaining it. the latest neo4j version used in [neo4j-tinkerpop-api-impl](https://github.com/neo4j-contrib/neo4j-tinkerpop-api-impl) was updated to 3.4.0 in november 2018; since then not much has happened. the feature implemented in this pull request would help us greatly in the project we are currently working in, and my intention was to share it with the community. as @spmallette pointed out, implementing it required changing some dependencies too, which is why new versions of [neo4j-tinkerpop-api](https://github.com/neo4j-contrib/neo4j-tinkerpop-api) and [neo4j-tinkerpop-api-impl](https://github.com/neo4j-contrib/neo4j-tinkerpop-api-impl) were released a few days ago. however, upgrading neo4j to the latest version (3.5.1) is quite another matter. i am afraid i will not be able to do it in the next days. i will fix the build (regarding enforcer problems) and update the pull request though. [ Full content available at: https://github.com/apache/tinkerpop/pull/1042 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
[GitHub] [tinkerpop] nastra opened pull request #1045: TINKERPOP-2141: Rewind buffer's position in ByteBufferSerializer
[ Full content available at: https://github.com/apache/tinkerpop/pull/1045 ] This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org
