Summary

Fix error message when Python isn't properly configured to proxy server.

Review Request #7689 — Created Oct. 10, 2015 and discarded Jan. 29, 2019, 1:41 p.m.

Information

Owner

calvinjshin

Repository

Review Board

Branch

master

Bugs

3971

Depends On

Reviewers

Groups

reviewboard

People

Description

Better handled 'URLError' error message by placing line 'data = e.read()' in try block instead of outside of it. This way, when there is a URLError exception, it will not raise an error with URLError calling read().

Testing Done

Tested by reloading browser (Chrome).

Ran ./reviewboard/manage.py test -- reviewboard.hostingsvcs.tests and all 167 tests passed.

Issues

Description	From	Last Updated
Should probably put this newline back	mike_conley	Oct. 10, 2015, 3:53 p.m.
'KilnClient' imported but unused	reviewbot	Oct. 14, 2015, 9:44 p.m.
Thanks for the test! This is fantastic. Is it possible to write a similar test for the GitHub hosting service?	mike_conley	Oct. 14, 2015, 9:55 p.m.
Please remove this line (so there's no blank line at the top of the function).	david	Oct. 15, 2015, 6:47 p.m.
Please remove this line.	david	Oct. 15, 2015, 6:47 p.m.
Here too.	david	Oct. 15, 2015, 6:48 p.m.
Can you sort these in alphabetical order?	david	Oct. 15, 2015, 6:49 p.m.
Should be "GitHub".	david	Oct. 15, 2015, 6:50 p.m.
This isn't testing anything. You're never actually calling any of the functions. It's not even taking all the required arguments. …	chipx86	Oct. 17, 2015, 3:15 p.m.
Same here.	chipx86	Oct. 17, 2015, 3:15 p.m.
Col: 80 E501 line too long (97 > 79 characters)	reviewbot	Nov. 21, 2015, 1:14 a.m.
Col: 80 E501 line too long (101 > 79 characters)	reviewbot	Nov. 21, 2015, 1:14 a.m.
Col: 80 E501 line too long (110 > 79 characters)	reviewbot	Nov. 21, 2015, 1:14 a.m.
Col: 80 E501 line too long (114 > 79 characters)	reviewbot	Nov. 21, 2015, 1:14 a.m.
Col: 80 E501 line too long (84 > 79 characters)	reviewbot	Nov. 21, 2015, 1:14 a.m.
Col: 9 E265 block comment should start with '# '	reviewbot	Nov. 21, 2015, 1:14 a.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Nov. 21, 2015, 1:14 a.m.
Col: 13 E116 unexpected indentation (comment)	reviewbot	Nov. 21, 2015, 1:14 a.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Nov. 21, 2015, 1:14 a.m.
Col: 80 E501 line too long (97 > 79 characters)	reviewbot	Dec. 4, 2015, 11:18 p.m.
Col: 80 E501 line too long (101 > 79 characters)	reviewbot	Dec. 4, 2015, 11:18 p.m.
Col: 80 E501 line too long (110 > 79 characters)	reviewbot	Dec. 4, 2015, 11:18 p.m.
Col: 80 E501 line too long (114 > 79 characters)	reviewbot	Dec. 4, 2015, 11:18 p.m.
Col: 80 E501 line too long (84 > 79 characters)	reviewbot	Dec. 4, 2015, 11:18 p.m.
Col: 80 E501 line too long (88 > 79 characters)	reviewbot	Dec. 4, 2015, 11:18 p.m.
Col: 9 E265 block comment should start with '# '	reviewbot	Dec. 4, 2015, 11:18 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Dec. 4, 2015, 11:18 p.m.
Col: 13 E116 unexpected indentation (comment)	reviewbot	Dec. 4, 2015, 11:18 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Dec. 4, 2015, 11:18 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/hostingsvcs/github.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/hostingsvcs/github.py

I think there are still .read() usages in kiln.py as well - can we fix those too?

CA

calvinjshin Oct. 14, 2015, 10:39 p.m.
```
Fixed.
```

reviewboard/hostingsvcs/github.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Should probably put this newline back
```

Change Summary:

Fixed Mike's issues.

Branch:

dev/easyfix

master

Diff:

Revision 2 (+25 -5)

Show changes

	reviewboard/hostingsvcs/github.py
	reviewboard/hostingsvcs/kiln.py
	reviewboard/hostingsvcs/tests.py

Tool: Pyflakes
Processed Files:
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/kiln.py
    reviewboard/hostingsvcs/tests.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/kiln.py
    reviewboard/hostingsvcs/tests.py

reviewboard/hostingsvcs/tests.py (Diff revision 2)
The issue has been resolved. Show all issues
```
 'KilnClient' imported but unused
```

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

The issue has been resolved. Show all issues

Thanks for the test! This is fantastic. Is it possible to write a similar test for the GitHub hosting service?

Change Summary:

Added test for github, per Mike's request

Diff:

Revision 3 (+36 -5)

Show changes

	reviewboard/hostingsvcs/github.py
	reviewboard/hostingsvcs/kiln.py
	reviewboard/hostingsvcs/tests.py

Tool: Pyflakes
Processed Files:
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/kiln.py
    reviewboard/hostingsvcs/tests.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/kiln.py
    reviewboard/hostingsvcs/tests.py

```
Just a handful of trivial requests.
```
reviewboard/hostingsvcs/github.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Please remove this line (so there's no blank line at the top of the function).
```
reviewboard/hostingsvcs/github.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Please remove this line.
```
reviewboard/hostingsvcs/kiln.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Here too.
```
reviewboard/hostingsvcs/tests.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Can you sort these in alphabetical order?
```
reviewboard/hostingsvcs/tests.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Should be "GitHub".
```

Change Summary:

Fixed David's issues.

Diff:

Revision 4 (+34 -6)

Show changes

	reviewboard/hostingsvcs/github.py
	reviewboard/hostingsvcs/kiln.py
	reviewboard/hostingsvcs/tests.py

Tool: Pyflakes
Processed Files:
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/kiln.py
    reviewboard/hostingsvcs/tests.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/kiln.py
    reviewboard/hostingsvcs/tests.py

The change description should be a more high level overview of the change instead of indicating what lines were moved where. Have a look at https://www.reviewboard.org/docs/codebase/dev/writing-good-descriptions/

Other than that, this change looks fine to me :)

Fix it!

reviewboard/hostingsvcs/tests.py (Diff revision 4)

An issue was opened. Show all issues

This isn't testing anything. You're never actually calling any of the functions. It's not even taking all the required arguments. That leads me to believe this wasn't actually tested.

Make sure you trigger something that will raise the URLError. I want to see a working assertRaises, and an assertion that service.client.http_get was called.

reviewboard/hostingsvcs/tests.py (Diff revision 4)
An issue was opened. Show all issues
```
Same here.
```

Change Summary:

Added an "assertRaisesMessage"

Diff:

Revision 5 (+36 -6)

Show changes

	reviewboard/hostingsvcs/github.py
	reviewboard/hostingsvcs/kiln.py
	reviewboard/hostingsvcs/tests.py

Tool: Pyflakes
Processed Files:
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/kiln.py
    reviewboard/hostingsvcs/tests.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/kiln.py
    reviewboard/hostingsvcs/tests.py

Change Summary:

Fixed _check_api_error function in github.py and added unit test.

Diff:

Revision 6 (+47 -7)

Show changes

	reviewboard/hostingsvcs/github.py
	reviewboard/hostingsvcs/kiln.py
	reviewboard/hostingsvcs/tests.py

Fix it!

Tool: Pyflakes
Processed Files:
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/kiln.py
    reviewboard/hostingsvcs/tests.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/kiln.py
    reviewboard/hostingsvcs/tests.py

reviewboard/hostingsvcs/github.py (Diff revision 6)
An issue was opened. Show all issues
```
Col: 80
 E501 line too long (97 > 79 characters)
```
reviewboard/hostingsvcs/github.py (Diff revision 6)
An issue was opened. Show all issues
```
Col: 80
 E501 line too long (101 > 79 characters)
```
reviewboard/hostingsvcs/tests.py (Diff revision 6)
An issue was opened. Show all issues
```
Col: 80
 E501 line too long (110 > 79 characters)
```
reviewboard/hostingsvcs/tests.py (Diff revision 6)
An issue was opened. Show all issues
```
Col: 80
 E501 line too long (114 > 79 characters)
```
reviewboard/hostingsvcs/tests.py (Diff revision 6)
An issue was opened. Show all issues
```
Col: 80
 E501 line too long (84 > 79 characters)
```
reviewboard/hostingsvcs/tests.py (Diff revision 6)
An issue was opened. Show all issues
```
Col: 9
 E265 block comment should start with '# '
```
reviewboard/hostingsvcs/tests.py (Diff revision 6)
An issue was opened. Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```
reviewboard/hostingsvcs/tests.py (Diff revision 6)
An issue was opened. Show all issues
```
Col: 13
 E116 unexpected indentation (comment)
```
reviewboard/hostingsvcs/tests.py (Diff revision 6)
An issue was opened. Show all issues
```
Col: 13
 E265 block comment should start with '# '
```

Diff:

Revision 7 (+63 -7)

Show changes

	docs/bugfix_wrapup.txt
	reviewboard/hostingsvcs/github.py
	reviewboard/hostingsvcs/kiln.py
	reviewboard/hostingsvcs/tests.py

Fix it!

Tool: Pyflakes
Processed Files:
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/kiln.py
    reviewboard/hostingsvcs/tests.py

Ignored Files:
    docs/bugfix_wrapup.txt



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/hostingsvcs/github.py
    reviewboard/hostingsvcs/kiln.py
    reviewboard/hostingsvcs/tests.py

Ignored Files:
    docs/bugfix_wrapup.txt

reviewboard/hostingsvcs/github.py (Diff revision 7)
An issue was opened. Show all issues
```
Col: 80
 E501 line too long (97 > 79 characters)
```
reviewboard/hostingsvcs/github.py (Diff revision 7)
An issue was opened. Show all issues
```
Col: 80
 E501 line too long (101 > 79 characters)
```
reviewboard/hostingsvcs/tests.py (Diff revision 7)
An issue was opened. Show all issues
```
Col: 80
 E501 line too long (110 > 79 characters)
```
reviewboard/hostingsvcs/tests.py (Diff revision 7)
An issue was opened. Show all issues
```
Col: 80
 E501 line too long (114 > 79 characters)
```
reviewboard/hostingsvcs/tests.py (Diff revision 7)
An issue was opened. Show all issues
```
Col: 80
 E501 line too long (84 > 79 characters)
```
reviewboard/hostingsvcs/tests.py (Diff revision 7)
An issue was opened. Show all issues
```
Col: 80
 E501 line too long (88 > 79 characters)
```
reviewboard/hostingsvcs/tests.py (Diff revision 7)
An issue was opened. Show all issues
```
Col: 9
 E265 block comment should start with '# '
```
reviewboard/hostingsvcs/tests.py (Diff revision 7)
An issue was opened. Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```
reviewboard/hostingsvcs/tests.py (Diff revision 7)
An issue was opened. Show all issues
```
Col: 13
 E116 unexpected indentation (comment)
```
reviewboard/hostingsvcs/tests.py (Diff revision 7)
An issue was opened. Show all issues
```
Col: 13
 E265 block comment should start with '# '
```

Status:: Discarded