Cannot display review when adding reviewer with empty Directory

[jherland]

I'm playing with Critic at work, and when I created my first review, I added a colleague specifically to that review (since nobody has configured any filters yet). In the "Add Reviewer" dialog, I entered his user name, and left the Directory blank, and pressed "Add". The review was created and emails sent out, but when I tried to enter the review page, I got the "Darn! It seems we have a problem...", with the following email being sent to admin:

User: jherland
Method: GET
URL: http://lys-critic.cisco.com/r/1

Traceback (most recent call last):
File "/usr/share/critic/critic.py", line 707, in main
for fragment in result:
File "/usr/share/critic/page/showreview.py", line 654, in renderShowReview
row("Reviewers", renderReviewers, "Users responsible for reviewing the changes in this review.", right=False)
File "/usr/share/critic/page/showreview.py", line 358, in row
if callable(value): value(main_row.td('value', id=cellId, colspan=colspan).preformatted())
File "/usr/share/critic/page/showreview.py", line 465, in renderReviewers
filter_id, filter_user = filter_data[(fullname, original_path)]
KeyError: ('Morten Rolland', '/')

-- critic

I ended up logging into the database, and manually resetting the path from "" to "/", and that seems to have fixed the crash. However, I'm guessing the empty string shouldn't have been stored into the db in the first place...

[jensl]

The fix for that was pushed to master yesterday, actually. Was reviewed here: http://critic-review.org/r/86

[jherland]

Hmm. I saw that fix, and believe I installed it before triggering the bug. Maybe I forgot to restart the server. Anyways, I'm unable to reproduce now, so obviously it is fixed. Sorry for the inconvenience.

BTW, how do you handle upgrading a running server? I've tried to re-run the install script after pulling some new commits, but it refuses (gracelessly) to install over the previous installation. For now, I figure out which files have changed, and then manually copy them one by one into /usr/share/critic, and finally I restart the services (apache and critic-main). This is obviously not very smart or safe...

[jensl]

Upgrading is done by running the upgrade.py script. I guess that might not be written down anywhere. So much isn't. :-)