Modify

Opened 6 years ago

Closed 6 years ago

#4211 closed defect (fixed)

bugs in recent commit

Reported by: mixedpuppy Owned by: otaku42
Priority: normal Component: TracPastePlugin
Severity: normal Keywords:
Cc: Trac Release: 0.11

Description

several upgrade issues in the current code, patch to be attached.

Attachments (1)

tracpaste-upgrade.patch (2.6 KB) - added by mixedpuppy 6 years ago.

Download all attachments as: .zip

Change History (7)

Changed 6 years ago by mixedpuppy

comment:1 follow-up: Changed 6 years ago by otaku42

  • Status changed from new to assigned

Thanks for the report, seems I messed things up a little. I will commit your patch in a minute, minus one thing:

In hunk 1 of your patch, you add current_version == self._get_version(db) again. That's not necessary, since it's already there (see line 38).

The rest of the patch is fine, and I wonder why I didn't hit any of these things in my testing.

comment:2 Changed 6 years ago by otaku42

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [4951]) TracPastePlugin: Fix bugs/shortcomings introduced in r4949, as
reported by mixedpuppy in #4211.

  • Wrap code to set/update tracpaste_version with a try/except, to catch exceptions. The changes made to that field are now also committed (doh!).
  • Add missing import of DatabaseManager class in _to_sql helper.
  • Use env directly where available, rather than referencing it via self.

Thanks a lot for the patch, mixedpuppy. Closes #4211.

comment:3 in reply to: ↑ 1 ; follow-up: Changed 6 years ago by mixedpuppy

  • Resolution fixed deleted
  • Status changed from closed to reopened

Replying to otaku42:

In hunk 1 of your patch, you add current_version == self._get_version(db) again. That's not necessary, since it's already there (see line 38).

That is necessary because in the loop above you might INSERT into the system table in one case. If the version is not retrieved again, you end up trying to INSERT into the system table again (rather than UPDATE), and that fails. This actually happened to me.

comment:4 Changed 6 years ago by mixedpuppy

Note, you also need to change the get call in the loop:

for function in version_map.get(version, []):

comment:5 in reply to: ↑ 3 Changed 6 years ago by otaku42

Replying to mixedpuppy:

That is necessary because in the loop above you might INSERT into the system table in one case.

You're right, it's required if the schema is updated from v1 to v2. Didn't notice that in the first place.

Replying to mixedpuppy:

Note, you also need to change the get call in the loop:

for function in version_map.get(version, []):

I see... so that get() returns [] if version is not in version_map. What puzzles me is the fact that at least Python 2.5 did not complain before (just checked that again).

comment:6 Changed 6 years ago by otaku42

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [4952]) TracPastePlugin: Fix remaining r4949 issues as reported by mixedpuppy
in #4211. Thanks again, mixedpuppy. Closes #4211.

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.