- Change Summary:
-
Gah, wrong branch. Sorry about that.
- Description:
-
A ReviewBoardServer's rb_version attribute is set via its check_api_version()
method. This is fine, but our init didn't set a default (dangerous) and if the check_api_version() lookup failed then the attribute simply didn't exist (broken). This caused stacktraces when the lookup fails, a user reports that this happens when his clock is out of sync causing authentication issues... Traceback (most recent call last):
File "/share/rbtools/rbtools/postreview.py", line 4114, in <module> main() File "/share/rbtools/rbtools/postreview.py", line 4081, in main if (parse_version(server.rb_version) >= parse_version('1.5.2') and AttributeError: 'ReviewBoardServer' object has no attribute 'rb_version' Adding a default parameter so check_api_version() failures will simply result
in version checks reporting that we're ancient. Also fixed a version check which was doing string comparison rather than comparing versions. This is available in the following branch of my repo...
~ https://github.com/atagar/rbtools/tree/newline_normalization ~ https://github.com/atagar/rbtools/tree/attr_defaulting
Defaulting rb_version attribute
Review Request #3209 — Created July 12, 2012 and submitted
A ReviewBoardServer's rb_version attribute is set via its check_api_version() method. This is fine, but our __init__ didn't set a default (dangerous) and if the check_api_version() lookup failed then the attribute simply didn't exist (broken). This caused stacktraces when the lookup fails, a user reports that this happens when his clock is out of sync causing authentication issues... Traceback (most recent call last): File "/share/rbtools/rbtools/postreview.py", line 4114, in <module> main() File "/share/rbtools/rbtools/postreview.py", line 4081, in main if (parse_version(server.rb_version) >= parse_version('1.5.2') and AttributeError: 'ReviewBoardServer' object has no attribute 'rb_version' Adding a default parameter so check_api_version() failures will simply result in version checks reporting that we're ancient. Also fixed a version check which was doing string comparison rather than comparing versions. This is available in the following branch of my repo... https://github.com/atagar/rbtools/tree/attr_defaulting
Tested by disabling my authentication credentials and running post-review for a perforce repository. We've been using this change against RBTools 0.3. I've adapted the change for the current master but this copy of it hasn't been exercised.
AT