Summary

New python API

Review Request #2508 — Created Aug. 3, 2011 and discarded Feb. 9, 2012, 9:43 a.m.

Information

Owner

mbait

Repository

RBTools

Branch

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

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

Testing Done

Issues

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 …	chipx86	Aug. 16, 2011, 7:25 a.m.
First line in the docs must be one line only. It should be a brief summary. Same with all other …	chipx86	Aug. 21, 2011, 9:12 p.m.
Not needed.	chipx86	Aug. 16, 2011, 7:24 a.m.
Two blank lines between these.	chipx86	Aug. 16, 2011, 7:24 a.m.
Two blank lines between these.	chipx86	Aug. 16, 2011, 7:24 a.m.
Wouldn't -1 just cause us to return the wrong error? Since it'd get the last entry in the array. I …	chipx86	Aug. 16, 2011, 7:23 a.m.
You can do this without the inspect module: for name, value in locals(): if value != RequestError and issubclass(value, RequestError): …	chipx86	Aug. 16, 2011, 7:21 a.m.
Still would rather not have FETCH_RQUEST_METHOD.	chipx86	Aug. 16, 2011, 7:32 a.m.
"Contains" and "requests"	chipx86	Aug. 16, 2011, 7:32 a.m.
One line.	chipx86	Aug. 16, 2011, 7:32 a.m.
"High"	chipx86	Aug. 16, 2011, 7:32 a.m.
Should use parens instead of \	chipx86	Aug. 16, 2011, 7:32 a.m.
All this should probably be put in another branch somewhere, instead of in here. Or it should just be added. …	chipx86	Aug. 20, 2011, 4:11 p.m.
Same with all these.	chipx86	Aug. 20, 2011, 4:11 p.m.
This function should be returning something, or it shouldn't be provided at all yet.	chipx86	Aug. 20, 2011, 4:11 p.m.
Blank line between these.	chipx86	Aug. 16, 2011, 7:35 a.m.
Blank line between these.	chipx86	Aug. 16, 2011, 7:35 a.m.
This can all probably be: setattr(resource, '__iter__', lambda: [i for i in lst])	chipx86	Aug. 20, 2011, 4:12 p.m.
The parameter should only be indented 4 spaces.	chipx86	Aug. 20, 2011, 4:12 p.m.
Swap these. variable == value	chipx86	Aug. 20, 2011, 4:14 p.m.
_iteritems The __ would cause the name to be mangled, making debugging harder.	chipx86	Aug. 20, 2011, 4:14 p.m.
return [i for i in self._iteritems]	chipx86	Aug. 20, 2011, 4:15 p.m.
How does this work asynchronously? There's no callback.	chipx86	Aug. 20, 2011, 4:15 p.m.
Capital D.	chipx86	Aug. 16, 2011, 7:37 a.m.
Should subclass object.	chipx86	Aug. 16, 2011, 7:39 a.m.

Groups:

+	rbtools

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.

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.

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.

rbtools/api/error.py (Diff revision 1)
Show all issues
```
Not needed.
```
rbtools/api/error.py (Diff revision 1)
Show all issues
```
Two blank lines between these.
```
rbtools/api/error.py (Diff revision 1)
Show all issues
```
Two blank lines between these.
```

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.

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

rbtools/api/request.py (Diff revision 1)

Show all issues

Still would rather not have FETCH_RQUEST_METHOD.

rbtools/api/request.py (Diff revision 1)
Show all issues
```
"Contains" and "requests"
```
rbtools/api/request.py (Diff revision 1)
Show all issues
```
One line.
```
rbtools/api/request.py (Diff revision 1)
Show all issues
```
"High"
```
rbtools/api/request.py (Diff revision 1)
Show all issues
```
Should use parens instead of \
```

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.

rbtools/api/request.py (Diff revision 1)
Show all issues
```
Same with all these.
```

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.

rbtools/api/resource.py (Diff revision 1)
Show all issues
```
Blank line between these.
```
rbtools/api/resource.py (Diff revision 1)
Show all issues
```
Blank line between these.
```

rbtools/api/resource.py (Diff revision 1)

Show all issues

This can all probably be:

setattr(resource, '__iter__', lambda: [i for i in lst])

rbtools/api/resource.py (Diff revision 1)
```
Is this used anywhere yet?
```

rbtools/api/resource.py (Diff revision 1)

Show all issues

The parameter should only be indented 4 spaces.

rbtools/api/resource.py (Diff revision 1)
Show all issues
```
Swap these. variable == value
```

rbtools/api/resource.py (Diff revision 1)

Show all issues

_iteritems

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

rbtools/api/resource.py (Diff revision 1)
Show all issues
```
return [i for i in self._iteritems]
```

rbtools/api/resource.py (Diff revision 1)

Show all issues

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

rbtools/api/server.py (Diff revision 1)
Show all issues
```
Capital D.
```
rbtools/api/server.py (Diff revision 1)
Show all issues
```
Should subclass object.
```

Description:

		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

Diff:

Revision 2 (+694 -989)

Show changes

	rbtools/api/errors.py
	rbtools/api/request.py
	rbtools/api/resource.py
	rbtools/api/server.py
	rbtools/api/serverinterface.py
	rbtools/api/tests.py

Review Board 6.0.3 alpha 0 (dev)

New python API

Information

Reviewers

Groups:

Description:

Diff:

Status: Discarded