You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by spmallette <gi...@git.apache.org> on 2017/02/28 18:56:59 UTC
[GitHub] tinkerpop pull request #565: TINKERPP-1612 Remove gremlin-groovy-test
GitHub user spmallette opened a pull request:
https://github.com/apache/tinkerpop/pull/565
TINKERPP-1612 Remove gremlin-groovy-test
https://issues.apache.org/jira/browse/TINKERPOP-1612
This pull request removes the `gremlin-groovy-test` module. Recall that these tests were deprecated in 3.2.4 and are good for removal at this point given changes to the testing approaches to GLVs. Removing it required a fairly heavy refactoring of `GremlinScriptEngine`, Gremlin Console, and Gremlin Server given that testing of a lot of the `GremlinScriptEngine` stuff was being accomplished through that module. As much of that infrastructure was moved down to `gremlin-core` it force a change of a lot of things. In short this PR touched a lot of stuff and there was really no way to do this in any smaller effort.
The old `GremlinPlugin` system of `gremlin-groovy` has been completely replaced by the more generalized system put in place in 3.2.4 (the old system was deprecated at that point). There is no more dual mode to the operations of the console.
Note that this PR also covers:
https://issues.apache.org/jira/browse/TINKERPOP-1421
https://issues.apache.org/jira/browse/TINKERPOP-1621
https://issues.apache.org/jira/browse/TINKERPOP-1622
which are related to removal of deprecated code in some way related to `GremlinScriptEngine` or "plugin" infrastructure.
There were a number of changes to documentation and the doc generation system in support of these changes. Docs generate without issue at this point using the new plugin system.
The build is good with `docker/build.sh -t -i -n` - VOTE +1
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/tinkerpop TINKERPOP-1612
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/565.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 #565
----
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #565: TINKERPOP-1612 Remove gremlin-groovy-test
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/565
I just pushed a fix on `tp32` which has merged forward to this branch. That should allow it to build for you now @okram - can you give it another shot?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #565: TINKERPOP-1612 Remove gremlin-groovy-test
Posted by davebshow <gi...@git.apache.org>.
Github user davebshow commented on the issue:
https://github.com/apache/tinkerpop/pull/565
`docker/build.sh -t -n -i` succeeded.
VOTE +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #565: TINKERPOP-1612 Remove gremlin-groovy-test
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/565
This is an insane amount of work you did here. I like it. We now have put Gremlin-Groovy and the same standing as Gremlin-Python. They are both simply dialects (variants) of Gremlin. This is superb.
I think once this gets merged, we should take the `gremlin-tools/` work I did in the `GraphActors` branch and put it here so we can reuse the `TinkerGraphProvider` for Groovy, Python, etc. I think that would be a nice cherry on top to this superb refactor.
VOTE +1.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #565: TINKERPOP-1612 Remove gremlin-groovy-test
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/tinkerpop/pull/565
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #565: TINKERPOP-1612 Remove gremlin-groovy-test
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/565
Gremlin-Python has failed twice for me locally now. :/ I'll keep reviewing the code aspects of Groovy.
```
[WARNING] Request for side-effect keys on b5f1b521-1431-4ee2-892d-191d3c47a500 returned no side-effects in the cache
[WARNING] Could not find side-effects for b5f1b521-1431-4ee2-892d-191d3c47a500.
[WARNING] Could not find side-effect key for x in fbeaa674-19f5-4629-9f11-289ff65a2c32.
[exec] tests/driver/test_driver_remote_connection.py ......
[exec] tests/driver/test_driver_remote_connection_threaded.py ..
[exec] tests/process/test_strategies.py ..
[exec] tests/process/test_traversal.py ...
[exec] tests/structure/test_graph.py ..
[exec] tests/structure/io/test_graphson.py ...........
[exec]
[exec] generated xml file: /Users/marko/software/tinkerpop/gremlin-python/target/python-reports/TEST-native-python.xml
[exec] =================================== FAILURES ===================================
[exec] ______________________________ test_client_async _______________________________
[exec]
[exec] client = <gremlin_python.driver.client.Client instance at 0x10b226cf8>
[exec]
[exec] def test_client_async(client):
[exec] g = Graph().traversal()
[exec] t = g.V()
[exec] message = RequestMessage('traversal', 'bytecode', {'gremlin': t.bytecode})
[exec] future = client.submitAsync(message)
[exec] > assert not future.done()
[exec] E assert not True
[exec] E + where True = <bound method Future.done of <Future at 0x10b016150 state=finished returned ResultSet>>()
[exec] E + where <bound method Future.done of <Future at 0x10b016150 state=finished returned ResultSet>> = <Future at 0x10b016150 state=finished returned ResultSet>.done
[exec]
[exec] tests/driver/test_client.py:75: AssertionError
[exec] ===================== 1 failed, 35 passed in 2.71 seconds ======================
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #565: TINKERPOP-1612 Remove gremlin-groovy-test
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/565
Test suite passes. Still doing a code review. Got stuck with the `ComputerVerificationStrategy` merge this afternoon...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---