You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by GitBox <gi...@apache.org> on 2020/08/12 07:48:23 UTC
[GitHub] [tinkerpop] rdtr opened a new pull request #1310: add getter method for GraphSONMessageSerializer
rdtr opened a new pull request #1310:
URL: https://github.com/apache/tinkerpop/pull/1310
Very simple changes. I just want a way to retrieve GraphSONMapper from these Serializer.
Background:
Our implementation is heavily inspired by [HttpGremlinEndpointHandler.java](https://github.com/apache/tinkerpop/blob/6c7be04be215f7877cca12b2d681d8cd81b7fb2f/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java) and we can retrieve MessageTextSerializer there, but this can only serialize / deserialize the whole response message.
I want to have capability to serialize / deserialize any element (e.g. some Vertex or Edge only) in the graph in HttpHandler and right now I define the same GraphSONMapper separately. But if I can just grab the GraphSONMapper I already setup for MessageTextSerializer, it would be good.
This mapper object can't be modified so it should also be safe. Please let me know if this addition is OK, if yes I will add tests + change log.
Thanks !
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1310: add getter method for GraphSONMessageSerializer
Posted by GitBox <gi...@apache.org>.
spmallette commented on pull request #1310:
URL: https://github.com/apache/tinkerpop/pull/1310#issuecomment-812510253
Merged and included 3ba462fca9a1fc43c559d1b328225bde649b6f96
--
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:
users@infra.apache.org
[GitHub] [tinkerpop] rdtr commented on pull request #1310: add getter method for GraphSONMessageSerializer
Posted by GitBox <gi...@apache.org>.
rdtr commented on pull request #1310:
URL: https://github.com/apache/tinkerpop/pull/1310#issuecomment-674979049
Thank you for the suggestion @spmallette . I am happy to update my code but could you elaborate how I can add `Mapper` implementation to GraphBinary ? Can I just put placeholder methods like `GraphML` ?
GraphSon has `ObjectMapper`, Gyro has `Kyro`, I am not sure what the counter of them for GraphBinary and what to expect to implement.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1310: add getter method for GraphSONMessageSerializer
Posted by GitBox <gi...@apache.org>.
spmallette commented on pull request #1310:
URL: https://github.com/apache/tinkerpop/pull/1310#issuecomment-677685005
Well, GraphBinary has `GraphBinaryReader` and `GraphBinaryWriter` so to make a `GraphBinaryMapper` implementation you'd have to create a class to hold both of those - perhaps that's just a `GraphBinaryReaderWriter` and that way you could do:
```java
public class GraphSONMapper implements Mapper<GraphBinaryReaderWriter> {
@Override
public GraphBinaryReaderWriter createMapper() {
....
}
```
Not sure what it looks like from there, but that would at least make all the MessageSerializers consistent for retrieving these sorts of things. Does that give you some idea on how to move forward?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette merged pull request #1310: add getter method for GraphSONMessageSerializer
Posted by GitBox <gi...@apache.org>.
spmallette merged pull request #1310:
URL: https://github.com/apache/tinkerpop/pull/1310
--
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:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1310: add getter method for GraphSONMessageSerializer
Posted by GitBox <gi...@apache.org>.
spmallette commented on pull request #1310:
URL: https://github.com/apache/tinkerpop/pull/1310#issuecomment-673432845
I can see why this change might be necessary, though I think my preference might be to return the TinkerPop `Mapper` object instead. In that way getting a "mapper" becomes generalized to the `MessageSerializer` interface. The `MessageSerializer` would have:
```java
Mapper getMapper();
```
and then you could just construct your own from that:
```java
Mapper<ObjectMapper> mapperObjectMapper = graphsonMessagerSerializer.getMapper();
ObjectMapper objectMapper = mapperObjectMapper.createMapper()
```
I imagine there are some generics to sort out in all this and I'm not sure if it can be done without breaking changes (so perhaps only a suitable change to 3.5.0). It would also require GraphBinary to have a `Mapper` implementation. There could be other problems too.
I just thought I'd mention this approach as it is a bit more robust than just adding getters for specific class types. Any thoughts on the matter?
----------------------------------------------------------------
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:
users@infra.apache.org