Opened 9 years ago

Closed 9 years ago

# bugs in recent commit

Reported by: Owned by: Shane Caraveo Michael Renzmann normal TracPastePlugin normal 0.11

### Description

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

### comment:1 follow-up:  3 Changed 9 years ago by Michael Renzmann

Status: new → 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 9 years ago by Michael Renzmann

Resolution: → fixed assigned → 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:  5 Changed 9 years ago by Shane Caraveo

Resolution: fixed closed → reopened

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 9 years ago by Shane Caraveo

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 9 years ago by Michael Renzmann

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.

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 9 years ago by Michael Renzmann

Resolution: → fixed reopened → closed

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

### Modify Ticket

Change Properties