Modify

Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#11664 closed defect (duplicate)

MySQL to PostgreSQL migration fails with codereviewer table

Reported by: Quince Owned by: Rob Guttman
Priority: high Component: CodeReviewerPlugin
Severity: major Keywords:
Cc: Ryan J Ollos Trac Release: 1.0

Description

fatt:/tmp/m # ./trac-migrate.py --in-place /srv/trac postgres://tracuser:xxxxxxxxx@127.0.0.1/trac?schema=trac

Copying tables:
  attachment table... 37 records.
  auth_cookie table... 4 records.
  cache table... 8 records.
  codereviewer table... Traceback (most recent call last):
  File "./trac-migrate.py", line 55, in <module>
    sys.exit(main(sys.argv[1:]) or 0)
  File "./trac-migrate.py", line 51, in main
    return TracMigrationCommand(env)._do_migrate(dest, dburi)
  File "/tmp/m/tracmigrate/admin.py", line 29, in _do_migrate
    return self._do_migrate_inplace(dburi)
  File "/tmp/m/tracmigrate/admin.py", line 63, in _do_migrate_inplace
    self._copy_tables(src_db, dst_db, src_dburi, dburi, inplace=True)
  File "/tmp/m/tracmigrate/admin.py", line 108, in _copy_tables
    @self._with_transaction(dst_db)
  File "/tmp/m/tracmigrate/admin.py", line 197, in wrapper
    fn(db)
  File "/tmp/m/tracmigrate/admin.py", line 143, in copy
    cursor.executemany(query, rows)
  File "/usr/lib/python2.7/site-packages/trac/db/util.py", line 85, in executemany
    return self.cursor.executemany(sql_escape_percent(sql), args)
psycopg2.DataError: integer out of range

No problem if I don't have CodeReviewerPlugin enabled, but if I have it disabled and enable it after the switch, it's no longer present in the interface and I can't set reviews even on new tickets.

Attachments (0)

Change History (12)

comment:1 Changed 11 years ago by Ryan J Ollos

It might be related to #10806. The time column should probably be int64 rather than integer.

We can also cleanup the code and use to_utimestamp(datetime.now(utc)) rather than: codereviewerplugin/0.12/coderev/model.py@13697:83#L81, and fix other places where trac.util.datefmt should be used.

comment:2 Changed 11 years ago by Quince

Actually, I'm the "Bubba" of #10806; I had forgotten my old username. What would be a workaround I could use at this time? I actually don't care if previous reviews are lost; I just need to be able to create new ones, and I don't know why the plugin GUI elements don't show up if I disable it and then reenable it post-migration (trac-admin upgrade seems to not recreate the plugin table, saying nothing's changed, even if I move the plugin egg out of the plugins directory before migration and then back in).

comment:3 Changed 11 years ago by Ryan J Ollos

There a row in the system table with name = coderev. You'll need to delete that entry so that trac-admin knows that the database needs to be upgraded.

comment:4 Changed 11 years ago by Jun Omae

#11663 was closed as a duplicate.

Root cause is that the plugin uses integer type for codereviewer.time and codereviewer_map.time.

The range of integer type in PostgreSQL is -2147483648 to +2147483647, see http://www.postgresql.org/docs/9.1/static/datatype-numeric.html.

comment:5 Changed 11 years ago by Quince

So would switching these to bigint and replacing the time format as per comment:1 be sufficient?

comment:6 Changed 11 years ago by Ryan J Ollos

Technically you'll want to use int64 for the Trac database API, but yes, if you don't care about migrating your old data, that should work. In fixing this for the plugin we'll need to take care to migrate data.

comment:7 Changed 11 years ago by anonymous

Last edited 11 years ago by Ryan J Ollos (previous) (diff)

comment:8 Changed 11 years ago by Ryan J Ollos

Use the Trac database API. For example: browser:/tags/trac-1.0.1/trac/db_default.py@:80,96#L75.

comment:9 Changed 11 years ago by Quince

If I change to the to_utimestamp(datetime.now(utc)) mentioned above, what do I change the pretty printing to codereviewerplugin/0.12/coderev/model.py@13697:200-203#L200?

Do I just need to change 203 to pretty_when += ' (%s ago)' % pretty_timedelta(from_utimestamp(when)) and the previous three lines are OK as is?

[Edit:] Or, actually, shouldn't need to change even 203? I can't tell because I'm not sure which ones are supposed to be in localtime and which in UTC. Also the other location, codereviewerplugin/0.12/coderev/util/reviewer.py@13697:94#L94 not clear if needs to be changed.

Last edited 10 years ago by Ryan J Ollos (previous) (diff)

comment:10 Changed 11 years ago by Quince

This seems to work, giving correct dates, as well as the migration. If someone wants to double-check it, perhaps you could merge it in the plugin:

  • coderev/model.py

     
    11import re
    22import time
    33
     4from datetime import datetime
    45from trac.mimeview import Context
    56from trac.resource import ResourceNotFound
    6 from trac.util.datefmt import pretty_timedelta
     7from trac.util.datefmt import pretty_timedelta, utc, from_utimestamp, to_utimestamp
    78from trac.wiki.formatter import format_to_html
    89from trac.ticket.model import Ticket
    910
     
    8081
    8182    def save(self, status, reviewer, summary, **kw):
    8283        status = self.encode(status)
    83         when = int(time.time() * self.EPOCH_MULTIPLIER)
     84        when = to_utimestamp(datetime.now(utc))
     85
    8486        if status == self.status and self._when != 0: # initial is special
    8587            status = '' # only save status when changed
    8688        if not status and not summary:
     
    200202            pretty_when = time.strftime('%Y-%m-%d %H:%M',
    201203                                        time.localtime(long(when) /
    202204                                                       self.EPOCH_MULTIPLIER))
    203             pretty_when += ' (%s ago)' % pretty_timedelta(when)
     205            pretty_when += ' (%s ago)' % pretty_timedelta(from_utimestamp(when))
    204206            summaries.append({
    205207                'repo': self.repo,
    206208                'changeset': self.changeset,
  • coderev/upgrades/db1.py

     
    99            Column('status', type='text'),
    1010            Column('reviewer', type='text'),
    1111            Column('summary', type='text'),
    12             Column('time', type='integer'),
     12            Column('time', type='int64'),
    1313            Index(columns=['repo', 'changeset', 'time']),
    1414        ],
    1515    ]
  • coderev/upgrades/db2.py

     
    77            Column('repo', type='text'),
    88            Column('changeset', type='text'),
    99            Column('ticket', type='text'),
    10             Column('time', type='integer'),
     10            Column('time', type='int64'),
    1111        ],
    1212    ]
    1313
  • coderev/util/sync.py

     
    88import re
    99import sys
    1010import sqlite3
     11from datetime import datetime
     12from trac.util.datefmt import utc, to_utimestamp
    1113from subprocess import Popen, STDOUT, PIPE
    1214
    1315EPOCH_MULTIPLIER = 1000000.0
     
    3234    for changeset_line in changeset_lines:
    3335        print '.',
    3436        rev,when,msg = changeset_line.split('|',2)
    35         when = long(when) * EPOCH_MULTIPLIER
     37        when = to_utimestamp(datetime.now(utc))
    3638        ticket_re = re.compile('#([0-9]+)')
    3739        for ticket in ticket_re.findall(msg):
    3840            try:
Last edited 11 years ago by Ryan J Ollos (previous) (diff)

comment:11 Changed 11 years ago by Ryan J Ollos

comment:10 looks like a good start. We need to created a new db3.py rather than modifying db1.py and db2.py so that existing installations are upgraded from db version 2 to db version 3.

comment:12 Changed 10 years ago by Ryan J Ollos

Resolution: duplicate
Status: newclosed

Closing as a duplicate of #10716.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Rob Guttman.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.