Code review

I'll start by saying that it is a stellar improvement wrt the old GraceDB client! Thank you Leo.

Some parts of the code have python3 only constructs (ex: super() ) so it should be stated that legacy python2 is not supported (and I am fine with it).

gracedb_sdk/__init__.py I don't think that multiple inheritance is the best mechanism here to construct the GraceDB client. I think that composition is more appropriate instead. Multiple inheritance is for 'IS-A' relationships, and composition for "HAS-A" relationships. And in our case, the client requires and owns a session to communicate with the GraceDB server. In addition, by using multiple inheritance for the session, it pollutes the attributes which map the REST resources and as a consequence, it artificially restricts on the client side what resource names can be defined server-side. Finally, by using composition, this somewhat awkward code would go away (since the Client and API classes would be one):

class API:
    def __init__(self, ...):
        ...
        self.client = self
        ...

gracedb_sdk/api/base.py:

• I know it's not going to be a CPU issue, but it may be simpler to compute the url in the init instead of an on-the-fly recursion. (it would avoid caching such as the one in BaseEvents.search)

• create_or_update: no else after return

• The class Resource can not be used as-is, since the url property requires the path attribute, which is not set. Moreover, the path attribute type is not consistent (not defined in Resource, instance attribute in ChildResource, class attribute otherwise). I would suggest the following (since python has no abstract properties):

class Resource:
   path = None
   def __init__(self, parent, path=None):
       if path is not None:
           self.path = path
       elif self.path is None:
           raise NotImplementedError('The path class attribute is not defined')

• ChildResource is not needed

• Deletable: make it a Mixin

• Terminology issue. Rename class HasChildResources to ListResource? The parent/child terminology is already used: logs is a child of an event, and an api can be decomposed as a family tree. Any resource could have children, not only those labelled as "HasChildResources", which refer to another concept: some resources can be containers of other resources. Instead of simply renaming, another suggestion is to have a mixin Listable for such container resources that would introduce a "resource_class" or "member_class" (instead of child_class) attribute and a getitem method.

• along the same lines, a "Mutable" mixin could be introduced for POST and PUT.

logs.py:

• these two lines can be removed

        data = (*field_collection('tagname', tags),
                *kwargs.items())