You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by xytxytxyt <gi...@git.apache.org> on 2017/08/23 15:12:05 UTC
[GitHub] tinkerpop pull request #698: added repeat simple path test
GitHub user xytxytxyt opened a pull request:
https://github.com/apache/tinkerpop/pull/698
added repeat simple path test
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/xytxytxyt/tinkerpop repeat-path-test
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/698.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 #698
----
commit ff2de55189d0af18c9aafa554e31af8d0b5c17f3
Author: Xian Teng <xi...@datastax.com>
Date: 2017-08-22T15:43:09Z
added repeat simple path test
----
---
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 #698: added repeat simple path test
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/698
The result is the same. However, I would prefer to have "the simplest query that fails" and thus, no `limit(1)`, but if the DSEGraph traversal strategies are failing cause of `limit(1)`, then we can include it. Perhaps you could test to see if you get the right result in DSEGraph without `limit(1)`.
```
graph.io(gryo()).readGraph('data/tinkerpop-modern.kryo')
g = graph.traversal()
```
---
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 #698: added repeat simple path test
Posted by okram <gi...@git.apache.org>.
Github user okram commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/698#discussion_r135646159
--- Diff: gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/branch/GroovyRepeatTest.groovy ---
@@ -97,5 +97,10 @@ public abstract class GroovyRepeatTest {
public Traversal<Vertex, Map<String, Long>> get_g_V_repeatXbothX_untilXname_eq_marko_or_loops_gt_1X_groupCount_byXnameX() {
new ScriptTraversal<>(g, "gremlin-groovy", "g.V.repeat(both()).until{it.get().value('name').equals('lop') || it.loops() > 1}.groupCount.by('name')")
}
+
+ @Override
+ public Traversal<Vertex, Path> get_g_V_hasXname_markoX_repeatXoutE_inV_simplePathX_untilXhasXname_rippleXX_path_byXnameX_byXlabelX() {
+ new ScriptTraversal<>(g, "gremlin-groovy", "g.V().has('name', 'marko').repeat(outE().inV().simplePath()).until(has('name', 'ripple')).limit(1).path().by(values('name')).by(T.label)")
--- End diff --
Good find. I'll tweak that accordingly on merge.
---
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 #698: added repeat simple path test
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/698
Merged. I had to tweak both tests a bit. `limit(1)` still in the groovy test. The test used `values('name')` instead of just `name`. Groovy test was not in sugar syntax.
---
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 #698: added repeat simple path test
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/698
Is the `limit(1)` necessary for the DSEGraph testing because against MODERN its the same result?There is only 1 outgoing path between marko and ripple.
```
gremlin> g.V().has('name', 'marko').repeat(outE().inV().simplePath()).until(has('name', 'ripple')).limit(1).path().by(values('name')).by(T.label)
==>[marko,knows,josh,created,ripple]
gremlin> g.V().has('name', 'marko').repeat(outE().inV().simplePath()).until(has('name', 'ripple')).path().by(values('name')).by(T.label)
==>[marko,knows,josh,created,ripple]
```
---
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 #698: added repeat simple path test
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/698
@xytxytxyt -- please close this ticket if everything looks good to you.
---
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 #698: added repeat simple path test
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/698
What is the intention of the test? What are you trying to prove?
---
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 #698: added repeat simple path test
Posted by xytxytxyt <gi...@git.apache.org>.
Github user xytxytxyt closed the pull request at:
https://github.com/apache/tinkerpop/pull/698
---
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 #698: added repeat simple path test
Posted by dkuppitz <gi...@git.apache.org>.
Github user dkuppitz commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/698#discussion_r135642930
--- Diff: gremlin-groovy-test/src/main/groovy/org/apache/tinkerpop/gremlin/process/traversal/step/branch/GroovyRepeatTest.groovy ---
@@ -97,5 +97,10 @@ public abstract class GroovyRepeatTest {
public Traversal<Vertex, Map<String, Long>> get_g_V_repeatXbothX_untilXname_eq_marko_or_loops_gt_1X_groupCount_byXnameX() {
new ScriptTraversal<>(g, "gremlin-groovy", "g.V.repeat(both()).until{it.get().value('name').equals('lop') || it.loops() > 1}.groupCount.by('name')")
}
+
+ @Override
+ public Traversal<Vertex, Path> get_g_V_hasXname_markoX_repeatXoutE_inV_simplePathX_untilXhasXname_rippleXX_path_byXnameX_byXlabelX() {
+ new ScriptTraversal<>(g, "gremlin-groovy", "g.V().has('name', 'marko').repeat(outE().inV().simplePath()).until(has('name', 'ripple')).limit(1).path().by(values('name')).by(T.label)")
--- End diff --
Not a big deal, but Groovy tests should use Gremlin sugar notation.
---
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 #698: added repeat simple path test
Posted by xytxytxyt <gi...@git.apache.org>.
Github user xytxytxyt commented on the issue:
https://github.com/apache/tinkerpop/pull/698
the short story is that there was a defect in dse graph exposed by this query, that luckily was caught because one of our tests happened to have it, but could be added to tinkerpop so that at least it can be caught earlier next time
so it's basically to plug a hole
more information: https://datastax.jira.com/browse/DSP-14204
---
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 #698: added repeat simple path test
Posted by xytxytxyt <gi...@git.apache.org>.
Github user xytxytxyt commented on the issue:
https://github.com/apache/tinkerpop/pull/698
not sure, since i don't know what in dse graph failed... what's the difference between having it there and not?
---
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 #698: added repeat simple path test
Posted by xytxytxyt <gi...@git.apache.org>.
Github user xytxytxyt commented on the issue:
https://github.com/apache/tinkerpop/pull/698
i removed the `limit(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 #698: added repeat simple path test
Posted by xytxytxyt <gi...@git.apache.org>.
Github user xytxytxyt commented on the issue:
https://github.com/apache/tinkerpop/pull/698
thanks!
---
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 #698: added repeat simple path test
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/698
All tests pass with `docker/build.sh -t -n -i`
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 #698: added repeat simple path test
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/698
VOTE +1. (I can do the final merge when all the votes are tallied)
---
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.
---