Summary

[WIP] HostingServices: add implementation of get_remote_repositories for Beanstalk

Review Request #5469 — Created Feb. 14, 2014 and discarded Jan. 10, 2015, 12:57 a.m.

Information

Owner

olessia

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard, students

People

chipx86

Description

HostingServices: add implementation of get_remote_repositories for Beanstalk

Testing Done

Manual testing

Issues

Description	From	Last Updated
Looking at _api_get, it looks like it will always return something, unless it raises an Exception. So are we waiting …	mike_conley	Feb. 16, 2014, 7:35 p.m.
No space before the """	chipx86	Feb. 28, 2014, 4:12 a.m.
This can probably just be one line.	chipx86	Feb. 28, 2014, 4:13 a.m.
Blank line before for loops.	chipx86	Feb. 28, 2014, 4:13 a.m.
type is a reserved word. For consistency, how about scm_type?	chipx86	Feb. 28, 2014, 4:13 a.m.
type is a reserved word.	chipx86	Feb. 28, 2014, 4:13 a.m.
We try to keep the statements simpler. Can you have a separate statement for setting mirror_path to None? Or just …	chipx86	Feb. 28, 2014, 4:14 a.m.
Blank lines around if statements.	chipx86	Feb. 23, 2014, 11:07 p.m.

Fix it!

Looking pretty solid so far - just one concern - see below.

reviewboard/hostingsvcs/beanstalk.py (Diff revision 1)

An issue was opened. Show all issues

Looking at _api_get, it looks like it will always return something, unless it raises an Exception.
So are we waiting for response to be the empty string here as our exit condition?
If that's the case, that sounds fragile (it makes us vulnerable to changes in Beanstalk's API that might put us in an infinite loop, if I'm understanding this correctly).

Change Summary:

The beanstalk api is currently limited, but they are working on version 2 that will have better support for pagination.

Testing Done:

~		Some manual testing
	~	Manual testing

Diff:

Revision 2 (+22)

Show changes

reviewboard/hostingsvcs/beanstalk.py

Groups:: reviewboard

Fix it!

reviewboard/hostingsvcs/beanstalk.py (Diff revision 2)
The issue has been resolved. Show all issues
```
No space before the """
```
reviewboard/hostingsvcs/beanstalk.py (Diff revision 2)
The issue has been resolved. Show all issues
```
This can probably just be one line.
```
reviewboard/hostingsvcs/beanstalk.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Blank line before for loops.
```
reviewboard/hostingsvcs/beanstalk.py (Diff revision 2)
The issue has been resolved. Show all issues
```
type is a reserved word. For consistency, how about scm_type?
```
reviewboard/hostingsvcs/beanstalk.py (Diff revision 2)
The issue has been resolved. Show all issues
```
type is a reserved word.
```

reviewboard/hostingsvcs/beanstalk.py (Diff revision 2)

The issue has been resolved. Show all issues

We try to keep the statements simpler. Can you have a separate statement for setting mirror_path to None?

Or just set it below when yielding, actually.

reviewboard/hostingsvcs/beanstalk.py (Diff revision 2)
An issue was opened. Show all issues
```
Blank lines around if statements.
```

Status:: Discarded