Summary

Support the new RawFileDiffData migration in condensediffs.

Review Request #6227 — Created Aug. 13, 2014 and submitted Aug. 21, 2014, 4:12 p.m.

Information

Owner

chipx86

Repository

Review Board

Branch

master

Bugs

Depends On

~~6226~~

Commit

0b8f843...

Reviewers

Groups

reviewboard

People

Description

condensediffs has been updated to handle the migration of
LegacyFileDiffData entries to RawFileDiffData. A lot of work went into
optimizing this. These are converted in batches, with minimal SQL
queries on a modern database (read: not sqlite3).

A run of condensediffs on a copy of a real-world database containing
650,000 diffs (all stored in LegacyFileDiffData entries) took 15 minutes
to convert, with a space savings of 80% (3.6GB down to 714MB).

Testing Done

Ran this many, many times without problems.

This was tested on sqlite, MySQL, and PostgreSQL.

Issues

Description	From	Last Updated
We should probably mark these strings for translation. Here and below.	david	Aug. 14, 2014, 5:03 p.m.
Since there's only one entry here, how about the single-line format? % {'count': total_count}	david	Aug. 14, 2014, 5:04 p.m.
You need to pass since into N_ here as a third argument (so it knows which one to choose)	david	Aug. 14, 2014, 5:49 p.m.
What if these tables already exist (say, a previous run of this failed in some unexpected way)?	david	Aug. 14, 2014, 5:16 p.m.

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/diffviewer/management/commands/condensediffs.py
    reviewboard/diffviewer/managers.py



Tool: Pyflakes
Processed Files:
    reviewboard/diffviewer/management/commands/condensediffs.py
    reviewboard/diffviewer/managers.py

reviewboard/diffviewer/management/commands/condensediffs.py (Diff revision 1)
The issue has been resolved. Show all issues
```
We should probably mark these strings for translation. Here and below.
```

reviewboard/diffviewer/management/commands/condensediffs.py (Diff revision 1)

The issue has been resolved. Show all issues

Since there's only one entry here, how about the single-line format?

% {'count': total_count}

reviewboard/diffviewer/managers.py (Diff revision 1)

The issue has been resolved. Show all issues

What if these tables already exist (say, a previous run of this failed in some unexpected way)?

chipx86 Aug. 14, 2014, 5:07 p.m.

Then that sucks and we'll deal.

I thought about that but looked at what Django's doing. They do a bunch of these tests, and don't even attempt to handle the case of the tables already existing. I figure if it happens, we can help with that and address it then, but if Django hasn't hit issues with this so far, we're not likely to.

david Aug. 14, 2014, 5:10 p.m.
```
OK.
```

chipx86 Aug. 14, 2014, 5:16 p.m.

Okay, everything does support DROP TABLE IF EXISTS, so nevermind, I'll add it.

Change Summary:


Localized text.
Condensed a dictionary to one line.
Added some DROP TABLE IF EXISTS statements.

Commit:

032e3b7894a80ae1625399aa56221c6ba32f1612

0b8f843ac28ada5f0699cb1c1a42a9e096ef5807

Diff:

Revision 2 (+325 -100)

Show changes

	reviewboard/diffviewer/managers.py
	reviewboard/diffviewer/management/commands/condensediffs.py

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/diffviewer/management/commands/condensediffs.py
    reviewboard/diffviewer/managers.py



Tool: Pyflakes
Processed Files:
    reviewboard/diffviewer/management/commands/condensediffs.py
    reviewboard/diffviewer/managers.py

reviewboard/diffviewer/management/commands/condensediffs.py (Diff revisions 1 - 2)

The issue has been dropped. Show all issues

You need to pass since into N_ here as a third argument (so it knows which one to choose)

chipx86 Aug. 14, 2014, 5:50 p.m.

ungettext works that way, but ungettext_lazy works using this format. That's also how the other strings for hours, minutes, etc. work in this function.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to master (6909af3)