Re: [DISCUSS] Creating pattern steps to codify best practices
So, I've been doing some additional thinking about the ways that this could
work based on the comments below and have put my comments inline.
Dave
On Tue, Sep 22, 2020 at 6:05 AM Stephen Mallette
wrote:
> I added some thoughts inline below:
>
> On Fri, Sep 18, 2020 at 3:51 PM David Bechberger
> wrote:
>
> > Thanks for the detailed comments Stephen. I have addressed them inline
> > below.
> >
> > I did read the proposal from earlier and I think that we are in close
> > agreement with what we are trying to accomplish. I also fully support
> > Josh's comment on providing a mechanism for submitting a map of
> properties
> > as manually unrolling this all right now leads to a lot of potential for
> > error and a long messy traversal.
> >
> > I'm looking forward to this discussion on how to merge these two
> proposals.
> >
> >
> > 1. How would multi/meta-properties fit into the API you've proposed?
> >
> > My first thought here is that multi-properties would be represented as
> > lists in the map, e.g.
> >
> > {names: ['Dave', 'David']}
> >
> > and meta-properties would be represented as maps in the maps, e.g.
> >
> > {name: {first: 'Dave', last: 'Bechberger'}}
> >
> > I can't say I've thought through all the implications of this though so
> it
> > is an area we would need to explore.
> >
>
> The first implication that comes to mind is that it makes the assumption
> the user wants multi/meta-properties as opposed to a single cardinality of
> List in the first case and a Map as a property value in the second case. I
> suppose that graphs with a schema could resolve those assumptions but
> graphs that are schemaless would have a problem. The issue could be
> resolved by specialized configuration of "g" or per merge() step using a
> with() modulator I suppose but that goes into a yet another level of
> implications to consider. I've often wondered if the start point for
> getting types/schema into TP3 without a full rewrite would be in this form
> where Gremlin would be given hints as to what to expect as to the types of
> data it might encounter while traversing. Anyway, I'd be hesitant to go
> down paths that don't account for multi/metaproperties well. They are
> first class citizens in TP3 (with those hoping for extension of at least
> multiproperties to edges) and while I find them a constant annoyance for so
> many reasons, we're kinda stuck with them.
>
I agree we need to account for multi/meta properties as 1st class
citizens. This is my current thinking on the syntax for the situations we
have laid out so far:
*Merge*
g.mergeV('name', {'name': 'marko'}, {'name': 'marko', 'age': 29})
*Merge with id* g.mergeV('name', {T.id: 1}, {T.id: 1, 'name': 'Marko'})
*Merge with Meta Properties* g.mergeV('name', {T.id: 1}, {T.id: 1, 'name':
{'first': Marko', 'last': 'R'})
*Merge with Multi Properties * g.mergeV('name', {T.id: 1}, {T.id: 1,
'lang': ['java', 'scala'])
*Merge with Single Cardinality* g.mergeV('name', {T.id: 1}, {T.id: 1,
'lang': 'java')
*Merge with List Cardinality* g.mergeV('name', {T.id: 1}, {T.id: 1, 'lang':
['java', 'scala', 'java'])
*Merge with Set Cardinality * g.mergeV('name', {T.id: 1}, {T.id: 1, 'lang':
['java', 'scala'])
Since in a mergeV() scenario we are only ever adding whatever values are
passed in there would be no need to specify the cardinality of the property
being added. If they wanted to add a value to an existing property then
the current property() method would still be available on the output
traversal. e.g.
g.mergeV('name', {T.id: 1}, {T.id: 1, 'lang': ['java', 'scala',
'java']).property(Cardinality.list, 'lang, 'java')
*Merge with stream * g.inject([{id: 1, label: 'person', 'name': 'marko'},
{id: 2, label: 'person', 'name': 'josh'}]).
mergeV(values('label'), valueMap('id'), valueMap())
*Merge with reporting added or updated side effect* g.mergeV('name',
{'name': 'marko'}, {'name': 'marko', 'age': 29}).
with(WithOptions.sideEffectLabel, 'a').
project('vertex', 'added').
by().
by(cap('a'))
>
>
> > 2. How would users set the T.id on creation? would that T.id just be a
> key
> > in the first Map argument?
> >
> > Yes, I was thinking that T.id would be the key name, e.g.:
> >
> > g.mergeV('name', {T.id: 1}, {'name': 'Marko'})
> >
>
> ok - I can't say I see a problem with that atm.
>
>
> > 3. I do like the general idea of a match on multiple properties for the
> > first argument as a convenience but wonder about the specificity of this
> > API a bit as it focuses heavily on equality - I suppose that's most cases
> > for get-or-create, so perhaps that's ok.
> >
> > In most cases I've seen use exact matches on the vertex or edge. I think
> > it might be best to keep this straightforward as any complex edge cases
> > still can perform the same functionality using the coalesce() pattern.
> >
>
> I played around with the idea to use modulators to apply additional
> constraints:
>
> g.mergeV('person', {'state':'NY'}, {'active': true}).
> by(ha
[jira] [Commented] (TINKERPOP-2427) Simplify Netty reference counting
[ https://issues.apache.org/jira/browse/TINKERPOP-2427?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17203459#comment-17203459 ] ASF GitHub Bot commented on TINKERPOP-2427: --- divijvaidya merged pull request #1334: URL: https://github.com/apache/tinkerpop/pull/1334 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Simplify Netty reference counting > - > > Key: TINKERPOP-2427 > URL: https://issues.apache.org/jira/browse/TINKERPOP-2427 > Project: TinkerPop > Issue Type: Improvement > Components: driver >Affects Versions: 3.5.0, 3.4.8 >Reporter: Divij Vaidya >Priority: Minor > > We have some incorrect configuration in the code such as: > [https://github.com/apache/tinkerpop/blob/3.4-dev/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/handler/WebSocketGremlinResponseDecoder.java#L59] > (MessageToMessageDecoder automatically decreases the reference count). > [https://github.com/apache/tinkerpop/blob/3.4-dev/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Handler.java#L270] > (SimpleChannelInboundHandler automatically decreases the reference count). > which are fixed by hardcoding values such as: > [https://github.com/apache/tinkerpop/blob/3.4-dev/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/handler/WebSocketClientHandler.java#L89] > This Jira will fix the reference counting correctly and eliminate the need > for hardcoding. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (TINKERPOP-2427) Simplify Netty reference counting
[ https://issues.apache.org/jira/browse/TINKERPOP-2427?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17203234#comment-17203234 ] ASF GitHub Bot commented on TINKERPOP-2427: --- spmallette commented on pull request #1334: URL: https://github.com/apache/tinkerpop/pull/1334#issuecomment-77900 Glad you were able to improve this code. The `ctx.fireChannelRead(frame.retain(2));` always bugged me. I'd recommend that you merge to `master` and run tests before merging this as I think you will find some conflict there VOTE +1 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Simplify Netty reference counting > - > > Key: TINKERPOP-2427 > URL: https://issues.apache.org/jira/browse/TINKERPOP-2427 > Project: TinkerPop > Issue Type: Improvement > Components: driver >Affects Versions: 3.5.0, 3.4.8 >Reporter: Divij Vaidya >Priority: Minor > > We have some incorrect configuration in the code such as: > [https://github.com/apache/tinkerpop/blob/3.4-dev/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/handler/WebSocketGremlinResponseDecoder.java#L59] > (MessageToMessageDecoder automatically decreases the reference count). > [https://github.com/apache/tinkerpop/blob/3.4-dev/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Handler.java#L270] > (SimpleChannelInboundHandler automatically decreases the reference count). > which are fixed by hardcoding values such as: > [https://github.com/apache/tinkerpop/blob/3.4-dev/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/handler/WebSocketClientHandler.java#L89] > This Jira will fix the reference counting correctly and eliminate the need > for hardcoding. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (TINKERPOP-2425) Server closes HTTP connection for keepAlive as true
[ https://issues.apache.org/jira/browse/TINKERPOP-2425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17203226#comment-17203226 ] ASF GitHub Bot commented on TINKERPOP-2425: --- spmallette commented on pull request #1333: URL: https://github.com/apache/tinkerpop/pull/1333#issuecomment-69726 VOTE +1 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Server closes HTTP connection for keepAlive as true > --- > > Key: TINKERPOP-2425 > URL: https://issues.apache.org/jira/browse/TINKERPOP-2425 > Project: TinkerPop > Issue Type: Bug > Components: server >Affects Versions: 3.5.0, 3.4.8 >Reporter: Divij Vaidya >Priority: Minor > > *Current behaviour* > In `HttpGremlinEndpointHandler` we close the connection when sending out an > error in the response even if keepAlive is set as true on the request. > *Expected behaviour* > Respect the `keepAlive` flag on the request and do not close the HTTP > connection if it set to true. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Commented] (TINKERPOP-2425) Server closes HTTP connection for keepAlive as true
[ https://issues.apache.org/jira/browse/TINKERPOP-2425?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17203220#comment-17203220 ] ASF GitHub Bot commented on TINKERPOP-2425: --- spmallette commented on a change in pull request #1333: URL: https://github.com/apache/tinkerpop/pull/1333#discussion_r495926402 ## File path: gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java ## @@ -82,19 +84,11 @@ import java.util.stream.Stream; import static com.codahale.metrics.MetricRegistry.name; -import static io.netty.handler.codec.http.HttpHeaderNames.ACCESS_CONTROL_ALLOW_ORIGIN; -import static io.netty.handler.codec.http.HttpHeaderNames.CONNECTION; -import static io.netty.handler.codec.http.HttpHeaderNames.CONTENT_LENGTH; -import static io.netty.handler.codec.http.HttpHeaderNames.CONTENT_TYPE; -import static io.netty.handler.codec.http.HttpHeaderNames.ORIGIN; +import static io.netty.handler.codec.http.HttpHeaderNames.*; import static io.netty.handler.codec.http.HttpMethod.GET; import static io.netty.handler.codec.http.HttpMethod.POST; -import static io.netty.handler.codec.http.HttpResponseStatus.BAD_REQUEST; -import static io.netty.handler.codec.http.HttpResponseStatus.CONTINUE; -import static io.netty.handler.codec.http.HttpResponseStatus.INTERNAL_SERVER_ERROR; -import static io.netty.handler.codec.http.HttpResponseStatus.METHOD_NOT_ALLOWED; -import static io.netty.handler.codec.http.HttpResponseStatus.NOT_FOUND; -import static io.netty.handler.codec.http.HttpResponseStatus.OK; +import static io.netty.handler.codec.http.HttpResponseStatus.*; Review comment: your IDE got a little fancy with our imports 🙂 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > Server closes HTTP connection for keepAlive as true > --- > > Key: TINKERPOP-2425 > URL: https://issues.apache.org/jira/browse/TINKERPOP-2425 > Project: TinkerPop > Issue Type: Bug > Components: server >Affects Versions: 3.5.0, 3.4.8 >Reporter: Divij Vaidya >Priority: Minor > > *Current behaviour* > In `HttpGremlinEndpointHandler` we close the connection when sending out an > error in the response even if keepAlive is set as true on the request. > *Expected behaviour* > Respect the `keepAlive` flag on the request and do not close the HTTP > connection if it set to true. -- This message was sent by Atlassian Jira (v8.3.4#803005)
[jira] [Created] (TINKERPOP-2428) Improve the ability to write tests with the Java driver
Stephen Mallette created TINKERPOP-2428:
---
Summary: Improve the ability to write tests with the Java driver
Key: TINKERPOP-2428
URL: https://issues.apache.org/jira/browse/TINKERPOP-2428
Project: TinkerPop
Issue Type: Improvement
Components: driver
Affects Versions: 3.4.8
Reporter: Stephen Mallette
We often see folks struggling to write tests with the the Java driver as it is
hard to mock functionality in there. The key problem is around the {{final}} in
{{ResultSet}} and the complexity of the constructor that it has. If there were
some simplifications there it seems like we'd be able to mock a {{Client}}
without too much trouble.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
