• 
      

    New python API

    Review Request #2508 — Created Aug. 3, 2011 and discarded

    Information

    RBTools

    Reviewers

    New API is designed to provide simple and efficient access to RB resources, it employs 'factory' pattern and code generation to parse resource JSON structure and make pythons objects on the top of it. The code posted here is not complete as it turned out that heavy changes that mostly have subjects to change are hard to review and redo.
    
    Related discussion is here:
    https://groups.google.com/d/topic/reviewboard-dev/HFm7rz4QE2k/discussion
    
    For a while there are no tests, so you can run this code
    to ensure that the new API works:
    
    from textwrap import wrap
    
    from rbtools.api.server import RBServer
    from rbtools.api.settings import Settings
    
    
    server = RBServer('http://reviews.reviewboard.org/')
    root = server.get_root()
    
    width = 80
    review_requests = root.get_review_requests()
    for rr in review_requests:
        print '\n', (' ' + rr.summary + ' ').center(width, '='), '\n'
        for line in wrap(rr.description, width):
            print line
    
    
    Corresponding branch is at http://github.com/mbait/rbtools/tree/resources
    
     
    Description From Last Updated

    This should be automatically provided by Exception. Also, self.message is deprecated in 2.6, so we wouldn't want to use it …

    chipx86chipx86

    First line in the docs must be one line only. It should be a brief summary. Same with all other …

    chipx86chipx86

    Not needed.

    chipx86chipx86

    Two blank lines between these.

    chipx86chipx86

    Two blank lines between these.

    chipx86chipx86

    Wouldn't -1 just cause us to return the wrong error? Since it'd get the last entry in the array. I …

    chipx86chipx86

    You can do this without the inspect module: for name, value in locals(): if value != RequestError and issubclass(value, RequestError): …

    chipx86chipx86

    Still would rather not have FETCH_RQUEST_METHOD.

    chipx86chipx86

    "Contains" and "requests"

    chipx86chipx86

    One line.

    chipx86chipx86

    "High"

    chipx86chipx86

    Should use parens instead of \

    chipx86chipx86

    All this should probably be put in another branch somewhere, instead of in here. Or it should just be added. …

    chipx86chipx86

    Same with all these.

    chipx86chipx86

    This function should be returning something, or it shouldn't be provided at all yet.

    chipx86chipx86

    Blank line between these.

    chipx86chipx86

    Blank line between these.

    chipx86chipx86

    This can all probably be: setattr(resource, '__iter__', lambda: [i for i in lst])

    chipx86chipx86

    The parameter should only be indented 4 spaces.

    chipx86chipx86

    Swap these. variable == value

    chipx86chipx86

    _iteritems The __ would cause the name to be mangled, making debugging harder.

    chipx86chipx86

    return [i for i in self._iteritems]

    chipx86chipx86

    How does this work asynchronously? There's no callback.

    chipx86chipx86

    Capital D.

    chipx86chipx86

    Should subclass object.

    chipx86chipx86
    MB
    chipx86
    1. Lots of little stylistic and doc stuff. I'll probably want to take another look at the implementation. Can you provide the link on github to the latest version you have of resources.py so I can see it without the delete/replace lines?
      
      Also, I prefer errors.py, as it's what we use through both Review Board and Djblets.
    2. rbtools/api/error.py (Diff revision 1)
       
       
       
      Show all issues
      This should be automatically provided by Exception.
      
      Also, self.message is deprecated in 2.6, so we wouldn't want to use it anyway.
    3. rbtools/api/error.py (Diff revision 1)
       
       
       
       
      Show all issues
      First line in the docs must be one line only. It should be a brief summary.
      
      Same with all other docstrings.
    4. rbtools/api/error.py (Diff revision 1)
       
       
       
      Show all issues
      Not needed.
    5. rbtools/api/error.py (Diff revision 1)
       
       
       
       
      Show all issues
      Two blank lines between these.
    6. rbtools/api/error.py (Diff revision 1)
       
       
       
       
      Show all issues
      Two blank lines between these.
    7. rbtools/api/error.py (Diff revision 1)
       
       
       
       
       
      Show all issues
      Wouldn't -1 just cause us to return the wrong error? Since it'd get the last entry in the array.
      
      I imagine this shouldn't default to anything. That way it's totally the caller's fault if specifying an incorrect parameter.
    8. rbtools/api/error.py (Diff revision 1)
       
       
       
       
       
      Show all issues
      You can do this without the inspect module:
      
      for name, value in locals():
          if value != RequestError and issubclass(value, RequestError):
              request_errors[value.code] = value
    9. rbtools/api/request.py (Diff revision 1)
       
       
      Show all issues
      Still would rather not have FETCH_RQUEST_METHOD.
    10. rbtools/api/request.py (Diff revision 1)
       
       
      Show all issues
      "Contains" and "requests"
    11. rbtools/api/request.py (Diff revision 1)
       
       
       
       
      Show all issues
      One line.
    12. rbtools/api/request.py (Diff revision 1)
       
       
      Show all issues
      "High"
    13. rbtools/api/request.py (Diff revision 1)
       
       
       
       
      Show all issues
      Should use parens instead of \
    14. rbtools/api/request.py (Diff revision 1)
       
       
       
       
      Show all issues
      All this should probably be put in another branch somewhere, instead of in here. Or it should just be added. Right now, there's no way of knowing what the reasoning is.
    15. rbtools/api/request.py (Diff revision 1)
       
       
       
      Show all issues
      Same with all these.
    16. rbtools/api/request.py (Diff revision 1)
       
       
      Show all issues
      This function should be returning something, or it shouldn't be provided at all yet.
    17. rbtools/api/resource.py (Diff revision 1)
       
       
       
      Show all issues
      Blank line between these.
    18. rbtools/api/resource.py (Diff revision 1)
       
       
       
      Show all issues
      Blank line between these.
    19. rbtools/api/resource.py (Diff revision 1)
       
       
       
       
       
       
       
       
      Show all issues
      This can all probably be:
      
      setattr(resource, '__iter__', lambda: [i for i in lst])
    20. rbtools/api/resource.py (Diff revision 1)
       
       
       
      Is this used anywhere yet?
    21. rbtools/api/resource.py (Diff revision 1)
       
       
       
      Show all issues
      The parameter should only be indented 4 spaces.
    22. rbtools/api/resource.py (Diff revision 1)
       
       
      Show all issues
      Swap these. variable == value
    23. rbtools/api/resource.py (Diff revision 1)
       
       
      Show all issues
      _iteritems
      
      The __ would cause the name to be mangled, making debugging harder.
    24. rbtools/api/resource.py (Diff revision 1)
       
       
       
      Show all issues
      return [i for i in self._iteritems]
    25. rbtools/api/resource.py (Diff revision 1)
       
       
      Show all issues
      How does this work asynchronously? There's no callback.
    26. rbtools/api/server.py (Diff revision 1)
       
       
      Show all issues
      Capital D.
    27. rbtools/api/server.py (Diff revision 1)
       
       
      Show all issues
      Should subclass object.
    28. 
        
    MB
    MB
    MB
    Review request changed
    Status:
    Discarded