Summary

Make 'rb-site upgrade' replace the example crontab.

Review Request #6096 — Created July 10, 2014 and submitted July 11, 2014, 3:59 p.m.

Information

Owner

david

Repository

Review Board

Branch

release-2.0.x

Bugs

3435

Depends On

Commit

08daf25...

Reviewers

Groups

reviewboard

People

Description

Because we changed search backends from PyLucene to Haystack, users need to
change their crontabs to use the new indexing commands. This change makes it so
that we replace the example crontab in the site directory with the templated
version on upgrade.

Some changes had to be made to template processing to avoid making assumptions
about some of the data, since upgrade doesn't ask any questions about domain
name, site root, etc. These aren't required for the cron template, so it all
works OK.

Testing Done

Created a 1.7.x site directory and saw that the sample crontab listed our old
PyLucene-based indexing commands. Ran 'rb-site upgrade' with this code and saw
it replaced with the new Haystack management commands.

Issues

Description	From	Last Updated
import 'choice' from line 17 shadowed by loop variable	reviewbot	July 10, 2014, 6:12 p.m.
Blank line before this.	chipx86	July 11, 2014, 1:13 a.m.
Do we allow empty entries for the site root? I thought we required a value. If not, we should require …	chipx86	July 11, 2014, 1:16 a.m.
Maybe do this at a different time from the rebuild_index, so we don't swamp the database. (Do we need to …	chipx86	July 11, 2014, 1:58 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/cmdline/rbsite.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/cmdline/rbsite.py

reviewboard/cmdline/rbsite.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 import 'choice' from line 17 shadowed by loop variable
```

Commit:

70c146ac7d461ebaa87e825628196a8545ee275b

946813a5d57609b1b3e1ae8242a01254bc59fb21

Diff:

Revision 2 (+20 -8)

Show changes

reviewboard/cmdline/rbsite.py

Tool: Pyflakes
Processed Files:
    reviewboard/cmdline/rbsite.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/cmdline/rbsite.py

Can we also fix the crontab to do two more things:
1) Run rb-site manage /path/to/site cleanup (for session cleanup) periodically (once a day?)

2) Use the -a (age) argument for update_index. (I think the minimum value is 1 hour, so -a 1). Otherwise, update_index looks at all review requests in the database.
We'll probably want to rename the cron file for #1, but I think it's worth it.

reviewboard/cmdline/rbsite.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Blank line before this.
```

reviewboard/cmdline/rbsite.py (Diff revision 2)

The issue has been dropped. Show all issues

Do we allow empty entries for the site root? I thought we required a value. If not, we should require one, since we default to '/'.

david July 11, 2014, 1:16 a.m.

I'm not 100% sure, but I think it must be fine, since otherwise we'd get IndexErrors on the slice. This code doesn't introduce any new problems, though.

chipx86 July 11, 2014, 2:57 a.m.

I was trying to figure out the reason for this code change, since the old code worked perfectly fine, so long as that assumption was correct. The wizard definitely requires an answer. I figured you must have hit a bug somewhere with this. If such a bug exists, I'd rather fix it at the source rather than bandaid it here.

Commit:

946813a5d57609b1b3e1ae8242a01254bc59fb21

390b21ddfbd671a25e61547c27eb061a107dd25a

Diff:

Revision 3 (+25 -9)

Show changes

	reviewboard/cmdline/rbsite.py
	reviewboard/cmdline/conf/search-cron.conf.in

Tool: Pyflakes
Processed Files:
    reviewboard/cmdline/rbsite.py

Ignored Files:
    reviewboard/cmdline/conf/cron.conf.in



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/cmdline/rbsite.py

Ignored Files:
    reviewboard/cmdline/conf/cron.conf.in

reviewboard/cmdline/conf/cron.conf.in (Diff revision 3)

The issue has been resolved. Show all issues

Maybe do this at a different time from the rebuild_index, so we don't swamp the database.

(Do we need to do rebuild_index at all anymore, actually?)

Commit:

390b21ddfbd671a25e61547c27eb061a107dd25a

08daf25da2a0d55d88f80565edef3ff50ae6bbef

Diff:

Revision 4 (+26 -13)

Show changes

	reviewboard/cmdline/rbsite.py
	reviewboard/cmdline/conf/cron.conf.in
	reviewboard/cmdline/conf/search-cron.conf.in

Tool: Pyflakes
Processed Files:
    reviewboard/cmdline/rbsite.py

Ignored Files:
    reviewboard/cmdline/conf/cron.conf.in
    reviewboard/cmdline/conf/search-cron.conf.in



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/cmdline/rbsite.py

Ignored Files:
    reviewboard/cmdline/conf/cron.conf.in
    reviewboard/cmdline/conf/search-cron.conf.in

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-2.0.x (8c41e45)