Re: Upgrading from 1.4.20 to 2.0 gives error with values stored as long in database (fwd)

Bah, didn't send this to the list. Martin (via irc) told me the regexp
fix worked for his tracker. Ganesh you can try the patch included
and it should fix the issue with python3.

I have ticketed this at:

   https://issues.roundup-tracker.org/issue2551170?

and fixed at:

   https://sourceforge.net/p/roundup/code/ci/82f870433b18

I am not going to generate a script to fix the database for two
reasons:

1. the bug that generated the ID as a long rather than as a string was fixed
   at least 15 (1.2.0 oct 2006 or prior) years ago. So newer trackers won't
   have the issue.

2. this only shows up when displaying the history for tickets modified
   using un-fixed old code. So the runtime delay of the fixup at
   display time should only happen for older ticket that are less
   likely to be displayed.

-- rouilj

------- Forwarded Message
Date: Wed, 10 Nov 2021 11:45:16 -0500

In message <[email protected]>,
=?UTF-8?Q?Martin_=c3=96stlund?= writes:
> [...]
>My procedure was that I installed a new server and installed roundup 
>1.4.20 with the database from the currently running instance, and 
>started working my way through the upgrade documentation at 
>https://roundup.sourceforge.io/docs/upgrading.html
>
> [...]
>It seems to be a problem with displaying history, for example
>/tracker/user7 would give a stacktrace
>
>Templating Error
><class 'SyntaxError'>: invalid syntax (<string>, line 1)
>Debugging information follows
>
>1. While evaluating the standard:'context/history' expression on line 180
>
>...
>
>2. A problem occurred in your template "user.item.html".
>
>Redacted stacktrace to last few lines:
>   File 
>"/usr/local/lib/python3.6/site-packages/roundup/cgi/templating.py", line 
>1064, in history
>     history = self._klass.history(self._nodeid, skipquiet=(not showall))
>   File "/usr/local/lib/python3.6/site-packages/roundup/hyperdb.py", 
>line 1237, in history
>     for j in self.db.getjournal(self.classname, nodeid):
>   File 
>"/usr/local/lib/python3.6/site-packages/roundup/backends/rdbms_common.py", 
>line 1400, in getjournal
>     params = eval_import(params)
>   File 
>"/usr/local/lib/python3.6/site-packages/roundup/anypy/strings.py", line 
>145, in eval_import
>     v = eval(s)
>   File "<string>", line 1
>     ('issue', 5027L, 'assignedto')
>                   ^
>SyntaxError: invalid syntax
>
>I googled and found out that in python2 long integer literals were 
>indicated with an l or L suffix. In Python 3, ints and longs have been 
>merged into just int, which functions pretty much like long used to.
>
>I can see in postgresql that I indeed have values stored with L suffix.
>
>Someone on #roundup thought it would be worth to try to use 
>roundup-admin -i <tracker dir> exporrtables on v1.6.0 and then same 
>with importabbled with 2.0.0, but that gave me the same kind of error 
>when trying to import the tables back:

Ok, for those watching at home, we have been discussing this on the
irc://irc.oftc.net/#roundup channel.

Joseph Myers, if you have other ideas on how to fix this it would be
welcome.

I was confused as to how a history reference was calling into
strings.py. Seeing the writeup above gave me a clue.

Martin, try this patch to roundup/anypy/strings.py

diff -r c3dfc4977ec6 roundup/anypy/strings.py
- --- a/roundup/anypy/strings.py  Sun Nov 07 13:18:39 2021 -0500
+++ b/roundup/anypy/strings.py  Wed Nov 10 11:06:26 2021 -0500
@@ -142,7 +142,11 @@
 def eval_import(s):
     """Evaluate a Python-2-style value imported from a CSV file."""
     if _py3:
- -        v = eval(s)
+        try:
+            v = eval(s)
+        except SyntaxError:
+            v = eval(s.replace('L,', ','))
+
         if isinstance(v, str):
             return v.encode('iso-8859-1').decode('utf-8')
         elif isinstance(v, dict):

bascially wrap the eval and catch the SyntaxError caused by the
integer with the L suffix. If we have the error, change the string and
replace: 'L,' with ',' everywhere and re-eval. If it still fails it
will toss another syntax error and we will see it.

If you run rounup-admin importtables with this code, it should clean
up the database entries. There should be no L in the journal/history
values.  No cost will be paid at display time, it's all at import.

If you have a entries like ('issue', 5600L, 'status') in your database,
it will properly handle them but you will pay a cost at display time.

Using s.replce is a little dangerous because an L, occuring in a quoted
string will also be modified. Rather than s.replace using:

  v = eval(re.sub(r', ([0-9]+)L,',r', \1,', s))

could be used. It looks for ', <numbers>L,' which is more specific and
less likely to occur in the quoted elements of the tuple. There is
still a risk and it probably has more of a performance penalty though.

Martin can you try this change in both cases:

   database history entries have an integer with a trailing L
   importtables and verify that no trailing L is present

Anybody else have a smarter and less risky idea on how to fix this?
Building a parser for the tuple format sounds like a big job and
likely to have a bug or two.

Other thoughts?

--
				-- rouilj
John Rouillard
===========================================================================
My employers don't acknowledge my existence much less my opinions.