Summary

Resource specific base classes for new API

Review Request #3273 — Created Aug. 12, 2012 and submitted Aug. 27, 2012, 5:22 p.m.

Information

Owner

smacleod

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools

People

chipx86

Description

This provides the resource specific base classes necessary for full API functionality (Such as downloading diffs). A few extra classes have been included to provide convenience functions (mostly for file uploading).

Dependent on parent diff in /r/3184.

Testing Done

Uploaded file attachments to demo.reviewboard.org.
Downloaded diffs locally.

Issues

Description	From	Last Updated
"make" should be removed I think.	chipx86	Aug. 15, 2012, 10:18 a.m.
Would be nice to say whether the diff and parent_diff are string contents or file objects.	chipx86	Aug. 15, 2012, 10:19 a.m.
Food for thought. It'd be neat to have a decorator for classes that did this instead, so classes wouldn't have …	chipx86	Aug. 15, 2012, 1:46 p.m.
Like above, this should say what format it's expecting for the content.	chipx86	Aug. 15, 2012, 12:30 p.m.
Too many s's?	chipx86	Aug. 15, 2012, 12:32 p.m.
Here too.	chipx86	Aug. 15, 2012, 12:32 p.m.
No need to capitalize "Review Request"	chipx86	Aug. 15, 2012, 12:32 p.m.

rbtools/api/resource.py (Diff revision 1)
The issue has been resolved. Show all issues
```
"make" should be removed I think.
```

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

The issue has been resolved. Show all issues

Would be nice to say whether the diff and parent_diff are string contents or file objects.

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

The issue has been dropped. Show all issues

Food for thought. It'd be neat to have a decorator for classes that did this instead, so classes wouldn't have to know about RESOURCE_MAP.

smacleod Aug. 15, 2012, 12:33 p.m.

This was just a copy of the pattern from the Web API resources on RB's side. I like this idea, I'll look into it.

chipx86 Aug. 15, 2012, 12:42 p.m.

Didn't mean to make this an issue. You definitely don't need to do it this change :)

rbtools/api/resource.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Like above, this should say what format it's expecting for the content.
```
rbtools/api/resource.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Too many s's?
```
rbtools/api/resource.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Here too.
```
rbtools/api/resource.py (Diff revision 1)
The issue has been resolved. Show all issues
```
No need to capitalize "Review Request"
```

Change Summary:

Updated based on Christian's review. Fixed a couple bugs causing the file uploading not to work.

Testing Done:

Tested by uploading file attachments to demo.reviewboard.org

Diff:

Revision 2 (+143 -4)

Show changes

	rbtools/api/decode.py
	rbtools/api/resource.py

Ship it!

```
Ship It!
```

Change Summary:

Updated to new parent diff from /r/3184/. Removed WIP tag.

Summary:

-	[WIP] Resource specific base classes for new API
+	Resource specific base classes for new API

Description:

		This provides the resource specific base classes necessary for full API functionality (Such as downloading diffs). A few extra classes have been included to provide convenience functions (mostly for file uploading).

-		Any feedback on what other resource specific base classes should be implemented, and any desired convenience methods would be appreciated.
-
		Dependent on parent diff in /r/3184.

Testing Done:

~		Tested by uploading file attachments to demo.reviewboard.org
	~	Uploaded file attachments to demo.reviewboard.org.
	+	Downloaded diffs locally.

Diff:

Revision 3 (+143 -4)

Show changes

	rbtools/api/decode.py
	rbtools/api/resource.py

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to api (b3f6891)