[jira] [Commented] (TINKERPOP-1402) Impossible for graph implementations to provide a class resolver for Gryo IO

2016-08-16 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15422738#comment-15422738
 ] 

ASF GitHub Bot commented on TINKERPOP-1402:
---

Github user asfgit closed the pull request at:

https://github.com/apache/tinkerpop/pull/380


> Impossible for graph implementations to provide a class resolver for Gryo IO
> 
>
> Key: TINKERPOP-1402
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1402
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: structure
>Affects Versions: 3.2.1
>Reporter: Bryn Cooke
>Priority: Critical
> Fix For: 3.2.2
>
>
> As far as I can tell there is no way for a graph implementation to specify a 
> classresolver for the following code:
> {noformat}g.io(IoCore.gryo()).writer().create(){noformat}
> The problem is that inside the graph implementation we need to be able to do 
> this:
> {noformat}
> public  I io(final Io.Builder builder) {
>   Io io = 
> builder.graph(this).registry(MyRegistry.INSTANCE).classResolver(MyClassReolver.INSTANCE).create();
> {noformat}
> but only supplying a registry is supported.
> Other solutions could be to design GryoIo for extension so that it can be 
> wrapped or to change the signature of Graph#io to:
> {noformat}
> public default Io io(final Io.Builder builder)
> {noformat}
> I would probably go for the signature change, so the graph is responsible for 
> deciding the implementation that is returned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (TINKERPOP-1402) Impossible for graph implementations to provide a class resolver for Gryo IO

2016-08-16 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15422683#comment-15422683
 ] 

ASF GitHub Bot commented on TINKERPOP-1402:
---

Github user twilmes commented on the issue:

https://github.com/apache/tinkerpop/pull/380
  
VOTE: +1


> Impossible for graph implementations to provide a class resolver for Gryo IO
> 
>
> Key: TINKERPOP-1402
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1402
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: structure
>Affects Versions: 3.2.1
>Reporter: Bryn Cooke
>Priority: Critical
> Fix For: 3.2.2
>
>
> As far as I can tell there is no way for a graph implementation to specify a 
> classresolver for the following code:
> {noformat}g.io(IoCore.gryo()).writer().create(){noformat}
> The problem is that inside the graph implementation we need to be able to do 
> this:
> {noformat}
> public  I io(final Io.Builder builder) {
>   Io io = 
> builder.graph(this).registry(MyRegistry.INSTANCE).classResolver(MyClassReolver.INSTANCE).create();
> {noformat}
> but only supplying a registry is supported.
> Other solutions could be to design GryoIo for extension so that it can be 
> wrapped or to change the signature of Graph#io to:
> {noformat}
> public default Io io(final Io.Builder builder)
> {noformat}
> I would probably go for the signature change, so the graph is responsible for 
> deciding the implementation that is returned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (TINKERPOP-1402) Impossible for graph implementations to provide a class resolver for Gryo IO

2016-08-16 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15422657#comment-15422657
 ] 

ASF GitHub Bot commented on TINKERPOP-1402:
---

Github user dkuppitz commented on the issue:

https://github.com/apache/tinkerpop/pull/380
  
VOTE: +1


> Impossible for graph implementations to provide a class resolver for Gryo IO
> 
>
> Key: TINKERPOP-1402
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1402
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: structure
>Affects Versions: 3.2.1
>Reporter: Bryn Cooke
>Priority: Critical
> Fix For: 3.2.2
>
>
> As far as I can tell there is no way for a graph implementation to specify a 
> classresolver for the following code:
> {noformat}g.io(IoCore.gryo()).writer().create(){noformat}
> The problem is that inside the graph implementation we need to be able to do 
> this:
> {noformat}
> public  I io(final Io.Builder builder) {
>   Io io = 
> builder.graph(this).registry(MyRegistry.INSTANCE).classResolver(MyClassReolver.INSTANCE).create();
> {noformat}
> but only supplying a registry is supported.
> Other solutions could be to design GryoIo for extension so that it can be 
> wrapped or to change the signature of Graph#io to:
> {noformat}
> public default Io io(final Io.Builder builder)
> {noformat}
> I would probably go for the signature change, so the graph is responsible for 
> deciding the implementation that is returned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (TINKERPOP-1402) Impossible for graph implementations to provide a class resolver for Gryo IO

2016-08-16 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15422593#comment-15422593
 ] 

ASF GitHub Bot commented on TINKERPOP-1402:
---

Github user spmallette commented on the issue:

https://github.com/apache/tinkerpop/pull/380
  
Note that the build failure in travis is that 
`IdentityRemovalStrategyTest$PerformanceTest>TraversalStrategyPerformanceTest.shouldBeFaster:118
 null` nonsense


> Impossible for graph implementations to provide a class resolver for Gryo IO
> 
>
> Key: TINKERPOP-1402
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1402
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: structure
>Affects Versions: 3.2.1
>Reporter: Bryn Cooke
>Priority: Critical
> Fix For: 3.2.2
>
>
> As far as I can tell there is no way for a graph implementation to specify a 
> classresolver for the following code:
> {noformat}g.io(IoCore.gryo()).writer().create(){noformat}
> The problem is that inside the graph implementation we need to be able to do 
> this:
> {noformat}
> public  I io(final Io.Builder builder) {
>   Io io = 
> builder.graph(this).registry(MyRegistry.INSTANCE).classResolver(MyClassReolver.INSTANCE).create();
> {noformat}
> but only supplying a registry is supported.
> Other solutions could be to design GryoIo for extension so that it can be 
> wrapped or to change the signature of Graph#io to:
> {noformat}
> public default Io io(final Io.Builder builder)
> {noformat}
> I would probably go for the signature change, so the graph is responsible for 
> deciding the implementation that is returned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (TINKERPOP-1402) Impossible for graph implementations to provide a class resolver for Gryo IO

2016-08-15 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15421066#comment-15421066
 ] 

ASF GitHub Bot commented on TINKERPOP-1402:
---

GitHub user spmallette opened a pull request:

https://github.com/apache/tinkerpop/pull/380

TINKERPOP-1402 Added IoBuilder.onMapper() method.

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

Deprecated IoBuilder.registry() in the process of doing that. This should 
provide greater flexibility to Graph providers who can now not only supply a 
custom IoRegistry but also access other non-generalized features of a 
Mapper.Builder instance.

Tested with `mvn clean install`. I did a one-time test before changing 
TinkerGraph to stop using the deprecated `registry` method and the build worked 
fine so I think that even though we can't directly test the old `registry` 
method anymore, it still should be working ok if provides elect to stick with 
it.

VOTE +1

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/apache/tinkerpop TINKERPOP-1402

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/tinkerpop/pull/380.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 #380


commit f1617e202025db1ff6f94deb70c66f14772273bf
Author: Stephen Mallette 
Date:   2016-08-15T14:29:46Z

Added IoBuilder.onMapper() method.

Deprecated IoBuilder.registry() in the process of doing that. This should 
provide greater flexibility to Graph providers who can now not only supply a 
custom IoRegistry but also access other non-generalized features of a 
Mapper.Builder instance.




> Impossible for graph implementations to provide a class resolver for Gryo IO
> 
>
> Key: TINKERPOP-1402
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1402
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: structure
>Affects Versions: 3.2.1
>Reporter: Bryn Cooke
>Priority: Critical
> Fix For: 3.2.2
>
>
> As far as I can tell there is no way for a graph implementation to specify a 
> classresolver for the following code:
> {noformat}g.io(IoCore.gryo()).writer().create(){noformat}
> The problem is that inside the graph implementation we need to be able to do 
> this:
> {noformat}
> public  I io(final Io.Builder builder) {
>   Io io = 
> builder.graph(this).registry(MyRegistry.INSTANCE).classResolver(MyClassReolver.INSTANCE).create();
> {noformat}
> but only supplying a registry is supported.
> Other solutions could be to design GryoIo for extension so that it can be 
> wrapped or to change the signature of Graph#io to:
> {noformat}
> public default Io io(final Io.Builder builder)
> {noformat}
> I would probably go for the signature change, so the graph is responsible for 
> deciding the implementation that is returned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (TINKERPOP-1402) Impossible for graph implementations to provide a class resolver for Gryo IO

2016-08-09 Thread stephen mallette (JIRA)

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15413927#comment-15413927
 ] 

stephen mallette commented on TINKERPOP-1402:
-

+1 - just sent a DISCUSS thread referenced as a link on this ticket

> Impossible for graph implementations to provide a class resolver for Gryo IO
> 
>
> Key: TINKERPOP-1402
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1402
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: structure
>Affects Versions: 3.2.1
>Reporter: Bryn Cooke
>Priority: Critical
> Fix For: 3.2.2
>
>
> As far as I can tell there is no way for a graph implementation to specify a 
> classresolver for the following code:
> {noformat}g.io(IoCore.gryo()).writer().create(){noformat}
> The problem is that inside the graph implementation we need to be able to do 
> this:
> {noformat}
> public  I io(final Io.Builder builder) {
>   Io io = 
> builder.graph(this).registry(MyRegistry.INSTANCE).classResolver(MyClassReolver.INSTANCE).create();
> {noformat}
> but only supplying a registry is supported.
> Other solutions could be to design GryoIo for extension so that it can be 
> wrapped or to change the signature of Graph#io to:
> {noformat}
> public default Io io(final Io.Builder builder)
> {noformat}
> I would probably go for the signature change, so the graph is responsible for 
> deciding the implementation that is returned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (TINKERPOP-1402) Impossible for graph implementations to provide a class resolver for Gryo IO

2016-08-09 Thread Bryn Cooke (JIRA)

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15413861#comment-15413861
 ] 

Bryn Cooke commented on TINKERPOP-1402:
---

On reflection I'd use Consumer rather than UnaryOperator. We won't ever return 
a new builder, just call methods on the one we get passed.


> Impossible for graph implementations to provide a class resolver for Gryo IO
> 
>
> Key: TINKERPOP-1402
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1402
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: structure
>Affects Versions: 3.2.1
>Reporter: Bryn Cooke
>Priority: Critical
>
> As far as I can tell there is no way for a graph implementation to specify a 
> classresolver for the following code:
> {noformat}g.io(IoCore.gryo()).writer().create(){noformat}
> The problem is that inside the graph implementation we need to be able to do 
> this:
> {noformat}
> public  I io(final Io.Builder builder) {
>   Io io = 
> builder.graph(this).registry(MyRegistry.INSTANCE).classResolver(MyClassReolver.INSTANCE).create();
> {noformat}
> but only supplying a registry is supported.
> Other solutions could be to design GryoIo for extension so that it can be 
> wrapped or to change the signature of Graph#io to:
> {noformat}
> public default Io io(final Io.Builder builder)
> {noformat}
> I would probably go for the signature change, so the graph is responsible for 
> deciding the implementation that is returned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (TINKERPOP-1402) Impossible for graph implementations to provide a class resolver for Gryo IO

2016-08-09 Thread stephen mallette (JIRA)

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15413598#comment-15413598
 ] 

stephen mallette commented on TINKERPOP-1402:
-

OK - I'd like to think on it for today but if I don't think of anything better 
I'll suggest deprecation of {{registry}} on the mailing list in favor of 
{{onMapper}} (or a better name) and see if anyone has any issues.

> Impossible for graph implementations to provide a class resolver for Gryo IO
> 
>
> Key: TINKERPOP-1402
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1402
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: structure
>Affects Versions: 3.2.1
>Reporter: Bryn Cooke
>Priority: Critical
>
> As far as I can tell there is no way for a graph implementation to specify a 
> classresolver for the following code:
> {noformat}g.io(IoCore.gryo()).writer().create(){noformat}
> The problem is that inside the graph implementation we need to be able to do 
> this:
> {noformat}
> public  I io(final Io.Builder builder) {
>   Io io = 
> builder.graph(this).registry(MyRegistry.INSTANCE).classResolver(MyClassReolver.INSTANCE).create();
> {noformat}
> but only supplying a registry is supported.
> Other solutions could be to design GryoIo for extension so that it can be 
> wrapped or to change the signature of Graph#io to:
> {noformat}
> public default Io io(final Io.Builder builder)
> {noformat}
> I would probably go for the signature change, so the graph is responsible for 
> deciding the implementation that is returned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (TINKERPOP-1402) Impossible for graph implementations to provide a class resolver for Gryo IO

2016-08-09 Thread Bryn Cooke (JIRA)

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15413578#comment-15413578
 ] 

Bryn Cooke commented on TINKERPOP-1402:
---

I like the onMapper idea and also agree the registry method is redundant.
Trying to unify the serialization apis would seem to be a pretty tall order, 
onMapper gives implementations the required flexibility. I think that the 
registry should be deprecated.

> Impossible for graph implementations to provide a class resolver for Gryo IO
> 
>
> Key: TINKERPOP-1402
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1402
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: structure
>Affects Versions: 3.2.1
>Reporter: Bryn Cooke
>Priority: Critical
>
> As far as I can tell there is no way for a graph implementation to specify a 
> classresolver for the following code:
> {noformat}g.io(IoCore.gryo()).writer().create(){noformat}
> The problem is that inside the graph implementation we need to be able to do 
> this:
> {noformat}
> public  I io(final Io.Builder builder) {
>   Io io = 
> builder.graph(this).registry(MyRegistry.INSTANCE).classResolver(MyClassReolver.INSTANCE).create();
> {noformat}
> but only supplying a registry is supported.
> Other solutions could be to design GryoIo for extension so that it can be 
> wrapped or to change the signature of Graph#io to:
> {noformat}
> public default Io io(final Io.Builder builder)
> {noformat}
> I would probably go for the signature change, so the graph is responsible for 
> deciding the implementation that is returned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)


[jira] [Commented] (TINKERPOP-1402) Impossible for graph implementations to provide a class resolver for Gryo IO

2016-08-09 Thread stephen mallette (JIRA)

[ 
https://issues.apache.org/jira/browse/TINKERPOP-1402?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15413474#comment-15413474
 ] 

stephen mallette commented on TINKERPOP-1402:
-

I'm hesitant to introduce a breaking change to the structure API, but I see the 
problem (i'm also not sure I see how changing the signature helps - a graph 
would get one type of {{Builder}} and then just sorta throw it away for 
whatever {{Io}} instance it feels like returning? maybe i'm missing the point). 
The {{classResolver}} is something newer that didn't really have a place in the 
{{Io.Builder}} interface, especially since it's specific to Gryo and really 
doesn't have a similar concept anywhere else. 

I'm wondering if a quieter way to handle it would be to add another method to 
{{Io.Builder}}:

{code}
public Builder onMapper(final UnaryOperator 
mapper);
{code} 

Of course, that makes the point of the {{registry}} method a bit useless as 
then you could just assign the {{IoRegistry}} in the {{UnaryOperator}}.  Maybe 
the {{IoRegistry}} could somehow have some function in allowing a graph to 
register a {{ClassResolver}} there in a general way? 



> Impossible for graph implementations to provide a class resolver for Gryo IO
> 
>
> Key: TINKERPOP-1402
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1402
> Project: TinkerPop
>  Issue Type: Improvement
>  Components: structure
>Affects Versions: 3.2.1
>Reporter: Bryn Cooke
>Priority: Critical
>
> As far as I can tell there is no way for a graph implementation to specify a 
> classresolver for the following code:
> {noformat}g.io(IoCore.gryo()).writer().create(){noformat}
> The problem is that inside the graph implementation we need to be able to do 
> this:
> {noformat}
> public  I io(final Io.Builder builder) {
>   Io io = 
> builder.graph(this).registry(MyRegistry.INSTANCE).classResolver(MyClassReolver.INSTANCE).create();
> {noformat}
> but only supplying a registry is supported.
> Other solutions could be to design GryoIo for extension so that it can be 
> wrapped or to change the signature of Graph#io to:
> {noformat}
> public default Io io(final Io.Builder builder)
> {noformat}
> I would probably go for the signature change, so the graph is responsible for 
> deciding the implementation that is returned.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)