[GitHub] [tinkerpop] dkuppitz opened pull request #1087: TINKERPOP-2191 Implement EdgeLabelVerificationStrategy

["dkuppitz (GitHub)" <gi...@apache.org>]

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

Implemented `EdgeLabelVerificationStrategy` to allow providers to encourage (or enforce) users to always specify edge labels.

VOTE +1

[ Full content available at: https://github.com/apache/tinkerpop/pull/1087 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] robertdale commented on issue #1087: TINKERPOP-2191 Implement EdgeLabelVerificationStrategy

["robertdale (GitHub)" <gi...@apache.org>]

VOTE +1

[ Full content available at: https://github.com/apache/tinkerpop/pull/1087 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] spmallette commented on pull request #1087: TINKERPOP-2191 Implement EdgeLabelVerificationStrategy

["spmallette (GitHub)" <gi...@apache.org>]

This is good. A couple of questions: 

1. Can the message get any more helpful somehow? Maybe it should say more about "why" this is important? Just a suggestion: "Always specify edge labels which restrict traversal paths ensuring optimal performance - the following steps need edge labels: VertexStep(OUT,vertex)".
2. I guess my second question relates to my message - It looks like the strategy throws the exception as soon as it runs into a problem. Would it be better to collect all the problems at once and report them in batch for fixing?
3. I don't suppose we have a way to convert "VertexStep(OUT,vertex)` to simple `out()` do we? Maybe `GroovyTranslator`  somehow? would be nice to actually somehow show the user exactly where they messed up in code rather than the `Step` representation, but perhaps that's not possible. I sorta think `profile()` would be more user friendly that way if it could be done. Maybe not something for this PR, but just something to think about.

[ Full content available at: https://github.com/apache/tinkerpop/pull/1087 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] dkuppitz closed pull request #1087: TINKERPOP-2191 Implement EdgeLabelVerificationStrategy

["dkuppitz (GitHub)" <gi...@apache.org>]

[ pull request closed by dkuppitz ]

[ Full content available at: https://github.com/apache/tinkerpop/pull/1087 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] spmallette commented on pull request #1087: TINKERPOP-2191 Implement EdgeLabelVerificationStrategy

["spmallette (GitHub)" <gi...@apache.org>]

`logWarning()` is good - nice

[ Full content available at: https://github.com/apache/tinkerpop/pull/1087 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] spmallette commented on issue #1087: TINKERPOP-2191 Implement EdgeLabelVerificationStrategy

["spmallette (GitHub)" <gi...@apache.org>]

VOTE +1

[ Full content available at: https://github.com/apache/tinkerpop/pull/1087 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org

[GitHub] [tinkerpop] dkuppitz commented on pull request #1087: TINKERPOP-2191 Implement EdgeLabelVerificationStrategy

["dkuppitz (GitHub)" <gi...@apache.org>]

I took the first part of your suggestion, but I still let it fail/log as soon as it hits the first step with unspecified labels. Two reasons:

1. All (verification) strategies behave like that.
2. With the current `toString()` representation it could look odd if we add multiple steps (in the worst case they all look the same).

Regarding the String representation of steps: I totally agree that it should be more user-friendly, however, we should create a ticket to do that for all error messages at once.

[ Full content available at: https://github.com/apache/tinkerpop/pull/1087 ]
This message was relayed via gitbox.apache.org for dev@tinkerpop.apache.org