davebshow / goblin

A Python 3.5 rewrite of the TinkerPop 3 OGM Goblin
Other
93 stars 21 forks source link

Vertex Property Values #74

Closed rosenbrockc closed 7 years ago

rosenbrockc commented 7 years ago

Suppose I have a vertex type Restaurant:

loop = asyncio.get_event_loop()
app = loop.run_until_complete(goblin.Goblin.open(loop, 
                                                 get_hashable_id=get_hashable_id))
S = loop.run_until_complete(app.session())
R = Restaurant()
R.id = 32952
loop.run_until_complete(S.get_vertex(R))

This correctly get all of the regular properties. However, I have a subclass of VertexProperty called Address. It correctly grabs the set of locations:

{<Address(type=<goblin.properties.String object at 0x107e23da0>, value=murray),
 <Address(type=<goblin.properties.String object at 0x107e23da0>, value=provo)}

but the individual properties of each Address are all None:

assert R.locations("murray").uuid is None
# True

I traced the problem to https://github.com/davebshow/goblin/blob/master/goblin/session.py#L223.

Because I have db_name mappings, the key in the props dictionary are the db_name instead of the property name. So the __properties__.get returns None and the second valueMap request never fires.

When I add:

for okey, val in props.items():
    key = element.__mapping__.db_properties[okey][0]
    if isinstance(element.__properties__.get(key), VertexProperty):
        trav = self._g.V(
                    props['id']).properties(okey).valueMap(True)
        vert_prop = await trav.toList()
        new_props[key] = vert_prop

then it works and the extra traversal is triggered. Mind, I am using the official 2.0 release on PyPI, so my version actually looks like:

        for okey, val in props.items():
            key = element.__mapping__.db_properties[okey][0]
            if isinstance(getattr(element, key, None), VertexPropertyManager):
                vert_prop = await self._g.V(
                    props['id']).properties(okey).valueMap(True).toList()
                new_props[key] = vert_prop

and the message sent to gremlin server is:

!application/vnd.gremlin-v2.0+json{"requestId": {"@type": "g:UUID", "@value": "0f385674-85f5-4f90-99d7-58ed8b30df93"}, "processor": "traversal", "op": "bytecode", "args": {"aliases": {"g": "g"}, "gremlin": {"@type": "g:Bytecode", "@value": {"step": [["V", {"@type": "g:Int64", "@value": 32952}], ["properties", "restaurant.locations"], ["valueMap", true]]}}}}

Unfortunately, the actual query doesn't execute properly and I am at a loss as to the problem. In python I get:

.../lib/python3.5/site-packages/aiogremlin/driver/resultset.py in wrapper(self)
     14                 raise exception.GremlinServerError(
     15                     msg.status_code,
---> 16                     "{0}: {1}".format(msg.status_code, msg.message))
     17             msg = msg.data
     18         return msg

GremlinServerError: 500: id

But, when I execute the following in the gremlin console:

gremlin> g.V(32952).properties("restaurant.locations").valueMap(true).toList()
==>{address.zip=84632, key=restaurant.locations, address.city=Murray, id=oif-pfc-lc5, address.uuid=12345678-1234-5678-1234-567812345679, address.street=456 Ruby Ave., value=murray}
==>{address.zip=84606, key=restaurant.locations, address.city=Provo, id=pav-pfc-lc5, address.street=123 Thai St., address.uuid=12345678-1234-5678-1234-567812345678, value=provo

it works. Any ideas for the error? I am happy to submit a PR for the mapping issue, though you may have a better way to handle it.

rosenbrockc commented 7 years ago

As a follow-up, if I execute:

loop.run_until_complete(g.V(32952).properties("restaurant.locations").valueMap(True).toList())

in python, it correctly returns the list of dictionaries:

[{'address.city': 'Murray',
  'address.street': '456 Ruby Ave.',
  'address.uuid': {'@type': 'g:UUID',
   '@value': '12345678-1234-5678-1234-567812345679'},
  'address.zip': 84632,
  'id': {'@type': 'janusgraph:RelationIdentifier',
   '@value': {'@class': 'java.util.HashMap', 'value': 'oif-pfc-lc5'}},
  'key': 'restaurant.locations',
  'value': 'murray'},
 {'address.city': 'Provo',
  'address.street': '123 Thai St.',
  'address.uuid': {'@type': 'g:UUID',
   '@value': '12345678-1234-5678-1234-567812345678'},
  'address.zip': 84606,
  'id': {'@type': 'janusgraph:RelationIdentifier',
   '@value': {'@class': 'java.util.HashMap', 'value': 'pav-pfc-lc5'}},
  'key': 'restaurant.locations',
  'value': 'provo'}]

I do have an appropriate get_hashable_id defined for the id properties.

rosenbrockc commented 7 years ago

Rats! This is what actually fixes the mapping problem:

            if okey in element.__mapping__.db_properties:
                key = element.__mapping__.db_properties[okey][0]
            else:
                key = okey

need to first check that the mapping exists. Special attributes (like id) aren't in there...

davebshow commented 7 years ago

Hi Conrad. I wan't to look into this a bit more. I just moved and am currently without computer. I will be getting set up later today. I will get to this issue sometime tomorrow afternoon.

davebshow commented 7 years ago

How about something like:

key, _ = element.__mapping__.db_properties.get(key, key)

As far as the specific traversal failure, could you post a minimal example that reproduces the error?

davebshow commented 7 years ago

Lol that will fail, but you get the idea, maybe:

if key in element.__mapping__.db_properties:
    key, _ = element.__mapping__.db_properties[key]
rosenbrockc commented 7 years ago

I can submit a PR for this. The traversal failure was due to the case where key was not in the db_properties dictionary. Adding the check made that problem go away. The traversals fire correctly now and get the valueMap back. However, there may still be a problem with updating the vertex property. I'll update with more details on Friday.

davebshow commented 7 years ago

Ok. If the PR isn't coming until Friday I may go ahead and make these changes myself. I plan on releasing a new Goblin today.

rosenbrockc commented 7 years ago

I am excited for the new release. Unfortunately I am on campus today and my other laptop is the one that I do the development with goblin on. The PR for #75 would also only come on Friday; it's a quick fix as well, so you may want to do that too.

davebshow commented 7 years ago

No worries. I will make the fixes. Thanks a lot for reporting.