You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by davebshow <gi...@git.apache.org> on 2017/08/09 19:31:13 UTC
[GitHub] tinkerpop pull request #691: TINKERPOP-1747 Streamline inheritance for greml...
GitHub user davebshow opened a pull request:
https://github.com/apache/tinkerpop/pull/691
TINKERPOP-1747 Streamline inheritance for gremlin-python GrapnSON serializer classes
https://issues.apache.org/jira/browse/TINKERPOP-1747
This PR cleans up the code used to define GraphSON serializer classes in gremlin-python.
`GraphSONMessageSerializer` acts as a base class for `GraphSONSerializersV2d0` and `GraphSONSerializersV3d0` in a similar fashion, but with this change:
1. While `GraphSONMessageSerializer` can accept custom reader, writer, and version implementations in order to create a custom serializer, `GraphSONSerializersV2d0` and `GraphSONSerializersV3d0` cannot.
2. Instead `GraphSONSerializersV3d0` and `GraphSONSerializersV2d0` are in a sense implementations of custom serializers, and they always use `graphsonV2d0.GraphSONReader()`/`graphsonV2d0.GraphSONWriter()` and `graphsonV3d0.GraphSONReader()`/`graphsonV3d0.GraphSONWriter()` respectively.
As before, `GraphSONMessageSerializer` will default to the current version of GraphSON (v3 for TP 3.3.0) if no custom reader/writer implementation is passed. Defaults are now specified in a highly visible class attribute position using uppercase naming like class CONSTANTS.
All other changes, like using `super` to call parent class's init methods, are more stylistic, reflecting current Python standards for best practice.
I also added a couple small tests to make sure each serializer was assigned proper reader, writer classes as well as version information.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/tinkerpop TINKERPOP-1747
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/691.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 #691
----
commit 83206a2101a0f03c515d4e1829149ed14a330dc7
Author: davebshow <da...@gmail.com>
Date: 2017-08-09T19:14:44Z
cleaned up gremlin-python message serializer classes
----
---
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 #691: TINKERPOP-1747 Streamline inheritance for gremlin-pyth...
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:
https://github.com/apache/tinkerpop/pull/691
I am not too hip to the Python code anymore, but I do know that adding the GraphSON3 support was a little "sketchy". Thus, if this is a Python-esque improvement upon my non-Python skills, then great.
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 #691: TINKERPOP-1747 Streamline inheritance for gremlin-pyth...
Posted by davebshow <gi...@git.apache.org>.
Github user davebshow commented on the issue:
https://github.com/apache/tinkerpop/pull/691
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 #691: TINKERPOP-1747 Streamline inheritance for gremlin-pyth...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the issue:
https://github.com/apache/tinkerpop/pull/691
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 pull request #691: TINKERPOP-1747 Streamline inheritance for greml...
Posted by davebshow <gi...@git.apache.org>.
Github user davebshow closed the pull request at:
https://github.com/apache/tinkerpop/pull/691
---
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 #691: TINKERPOP-1747 Streamline inheritance for gremlin-pyth...
Posted by davebshow <gi...@git.apache.org>.
Github user davebshow commented on the issue:
https://github.com/apache/tinkerpop/pull/691
Yeah well there are different approaches we could have taken, e.g. leaving the custom reader/writer options for the v2 and v3 serializer classes, but it seems to me that the point of those classes is to provide consistency. In the less common use case where the user provides reader/writer I think that it is reasonably to expect that they can deal with all the required configuration using `GraphSONMessageSerializer`.
---
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.
---