Fix KeyError when using help()

[fridex]

When calling Python's help() on a mapped item, there is raised KeyError with objclass being the requested key.

[fridex]

The CI failure does not look like to be relevant here.

[fridex]

@davebshow, ping. Any chance to get this reviewed/in?

[davebshow]

Yes, busy week for me. I will take a look at this either today or tomorrow. thanks @fridex

[davebshow]

Hi @fridex. Could you please paste snippet that I can use to reproduce this error?

[fridex]

Sure:

$ cat reproducer.py 

import goblin

class Foo(goblin.Vertex):
    bar = goblin.VertexProperty(goblin.properties.String)

help(Foo)

$ python3 reproducer.py > /dev/null && echo OK

Traceback (most recent call last):
  File "/home/fpokorny/.local/lib/python3.6/site-packages/goblin/mapper.py", line 218, in __getattr__
    mapping, _ = self._ogm_properties[value]
KeyError: '__objclass__'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "reproducer.py", line 6, in <module>
    help(Foo)
  File "/usr/lib64/python3.6/_sitebuiltins.py", line 103, in __call__
    return pydoc.help(*args, **kwds)
  File "/usr/lib64/python3.6/pydoc.py", line 1857, in __call__
    self.help(request)
  File "/usr/lib64/python3.6/pydoc.py", line 1910, in help
    else: doc(request, 'Help on %s:', output=self._output)
  File "/usr/lib64/python3.6/pydoc.py", line 1644, in doc
    pager(render_doc(thing, title, forceload))
  File "/usr/lib64/python3.6/pydoc.py", line 1637, in render_doc
    return title % desc + '\n\n' + renderer.document(object, name)
  File "/usr/lib64/python3.6/pydoc.py", line 382, in document
    if inspect.isclass(object): return self.docclass(*args)
  File "/usr/lib64/python3.6/pydoc.py", line 1282, in docclass
    for name, kind, cls, value in classify_class_attrs(object)
  File "/usr/lib64/python3.6/pydoc.py", line 205, in classify_class_attrs
    for (name, kind, cls, value) in inspect.classify_class_attrs(object):
  File "/usr/lib64/python3.6/inspect.py", line 427, in classify_class_attrs
    homecls = getattr(get_obj, "__objclass__", homecls)
  File "/home/fpokorny/.local/lib/python3.6/site-packages/goblin/mapper.py", line 223, in __getattr__
    value, self._element_type))
goblin.exception.MappingError: unrecognized property __objclass__ for class: vertex

$ python3 -VV   
Python 3.6.3 (default, Oct  9 2017, 12:11:29) 
[GCC 7.2.1 20170915 (Red Hat 7.2.1-2)]

See docs.

[davebshow]

As I suspected this has to do with the fact that I didn't follow the docs when implementing the __getattr__ override. This fix we need addresses the root problem, something like this:

    def __getattr__(self, value):
        try:
            mapping, _ = self._ogm_properties[value]
            return mapping
        except:
            raise AttributeError from exception.MappingError(
                "unrecognized property {} for class: {}".format(value, self._element_type))

With this change, we can run your code:

In [1]: import goblin
   ...: 
   ...: class Foo(goblin.Vertex):
   ...:     bar = goblin.VertexProperty(goblin.properties.String)
   ...: 
   ...: help(Foo)
   ...: 
class Foo(goblin.element.Vertex)
 |  Base class for user defined Vertex classes
 |  
 |  Method resolution order:
 |      Foo
 |      goblin.element.Vertex
 |      goblin.element.Element
 |      builtins.object
 |  
 |  Data and other attributes defined here:
 |  
 |  __label__ = 'foo'
 |  
 |  __mapping__ = <Mapping(type=vertex, label=foo, properties={'ba...n.pro...
 |  
 |  __properties__ = {'bar': <VertexProperty(type=<goblin.properties.Strin...
 |  
 |  __type__ = 'vertex'
 |  
 |  bar = 'bar'
 |  
 |  ----------------------------------------------------------------------
 |  Methods inherited from goblin.element.Vertex:
 |  
 |  to_dict(self)
 |  
 |  ----------------------------------------------------------------------
 |  Class methods inherited from goblin.element.Vertex:
 |  
 |  from_dict(d) from goblin.element.ElementMeta
 |  
 |  ----------------------------------------------------------------------
 |  Methods inherited from goblin.element.Element:
 |  
 |  __init__(self, **kwargs)
 |      Initialize self.  See help(type(self)) for accurate signature.
 |  
 |  ----------------------------------------------------------------------
 |  Data descriptors inherited from goblin.element.Element:
 |  
 |  __dict__
 |      dictionary for instance variables (if defined)
 |  
 |  __weakref__
 |      list of weak references to the object (if defined)
 |  
 |  id

[fridex]

As I suspected this has to do with the fact that I didn't follow the docs when implementing the getattr override. This fix we need addresses the root problem, something like this:

Thanks. The PR was updated.

[davebshow]

Ok we can probably merge this. Let's get rid of the bare except though as well, while we are at it.

[davebshow]

Actually, no, what about the tests. No I am wondering a bit what is the best approach. Maybe MappingError should inherit from AttributeError. Let me think about this a bit

[davebshow]

And I am not talking about the CI, that is hosed, but that is a separate issue. I am talking about the tests that check for MappingError: https://github.com/davebshow/goblin/search?utf8=%E2%9C%93&q=MappingError&type=

[fridex]

I am wondering a bit what is the best approach. Maybe MappingError should inherit from AttributeError. Let me think about this a bit

If the mapping error is used only in this case where attribute error is expected, it would be better in my opinion to inherit from AttributeError instead of Exception.

[davebshow]

Ok, that sounds reasonable. MappingError will inherit from AttributeError. Could you please make the changes and verify the fix by manually running the goblin tests as well as checking that help indeed works as expected (it will). Thanks for working through this with me.

[fridex]

I've amended the PR. Running help() now works as expected. I didn't dig into test-suite, but it looks like it is either broker or it takes really long to finish so I was not able to verify.

[davebshow]

@fridex Thanks for this!