Fix mysterious scope loss of a radio button callback in rb-site wizard

Review Request #2374 — Created May 18, 2011 and submitted

Information

Review Board

Reviewers

As a student of GSoC'11 I got my first task was to install RB and fix trivial bug. During installation I ran rb-site and noticed strange behavior of the setup wizard. Namely, it didn't remember my choice of caching method and still tried to ask about memcached server location. Investigation the reason of this behavior I found out that the user choice wasn't actually stored as the button callback function lost its scope and hence coulnd't find the object to write. As the result each time I toggled a radio button I got 

Traceback (most recent call last):
  File "/home/mbait/src/reviewboard/reviewboard/cmdline/rbsite.py", line 1044, in on_toggled
    setattr(save_obj, save_var, valid_choices[radio_button])
NameError: free variable 'save_obj' referenced before assignment in enclosing scope

Incidentally I fixed some other possible corner case related errors
Manual testing
MB
  1. 
      
  2. reviewboard/cmdline/rbsite.py (Diff revision 1)
     
     
    I don't know why, but this was actually the cause of the problem. I guess we ran into some of the pythonic issues here.
  3. 
      
chipx86
  1. 
      
  2. reviewboard/cmdline/rbsite.py (Diff revision 1)
     
     
     
    Blank line between these.
    
    There should generally be a blank line between blocks of code (such as if statements).
  3. reviewboard/cmdline/rbsite.py (Diff revision 1)
     
     
     
    You can do:
    
    first_enabled += 1
    
    What happens if we three radio buttons, the first of which is enabled and the others which are not? First_enabled is going to end up being 2. Probably not what we want.
  4. reviewboard/cmdline/rbsite.py (Diff revision 1)
     
     
     
    Blank line between these too.
  5. reviewboard/cmdline/rbsite.py (Diff revision 1)
     
     
     
    And here.
  6. 
      
MB
Review request changed

Change Summary:

- added some blank lines between logical blocks
- fixed strategy of calculation the first enabled button

Diff:

Revision 2 (+15 -9)

Show changes

chipx86
  1. Thanks! Committed to master (1b95a4e)
  2. 
      
Loading...