Opened 13 years ago
Closed 8 years ago
#8792 closed defect (fixed)
[patch] ValueError: list.remove(x): x not in list
Reported by: | Marek | Owned by: | Ryan J Ollos |
---|---|---|---|
Priority: | highest | Component: | MasterTicketsPlugin |
Severity: | blocker | Keywords: | |
Cc: | marekjarosz@…, mwisnicki@…, rockethacker | Trac Release: | 0.12 |
Description
Someone's created a ticket with a huge number of blocked by.
Now, every time I try to change anything in this ticket I get an internal error:
Trac detected an internal error: ValueError: list.remove(x): x not in list
File "/usr/lib/python2.4/site-packages/Trac-0.12.2-py2.4.egg/trac/web/main.py", line 511, in _dispatch_request File "/usr/lib/python2.4/site-packages/Trac-0.12.2-py2.4.egg/trac/web/main.py", line 237, in dispatch File "/usr/lib/python2.4/site-packages/Trac-0.12.2-py2.4.egg/trac/ticket/web_ui.py", line 169, in process_request File "/usr/lib/python2.4/site-packages/Trac-0.12.2-py2.4.egg/trac/ticket/web_ui.py", line 541, in _process_ticket_request File "/usr/lib/python2.4/site-packages/Trac-0.12.2-py2.4.egg/trac/ticket/web_ui.py", line 1238, in _do_save File "/usr/lib/python2.4/site-packages/Trac-0.12.2-py2.4.egg/trac/ticket/model.py", line 360, in save_changes File "build/bdist.linux-x86_64/egg/mastertickets/api.py", line 111, in ticket_changed File "build/bdist.linux-x86_64/egg/mastertickets/model.py", line 68, in save File "build/bdist.linux-x86_64/egg/mastertickets/model.py", line 61, in <lambda>
Trac 0.12.2 Genshi 0.6 mod_python 3.2.8 psycopg2 2.0.8 (dec dt ext pq3) Pygments 1.3.1 Python 2.4.3 (#1, Sep 3 2009, 15:37:37) [GCC 4.1.2 20080704 (Red Hat 4.1.2-46)] pytz 2010l setuptools 0.6c12 Subversion 1.4.2 (r22196) jQuery 1.4.2
Attachments (0)
Change History (23)
comment:2 Changed 12 years ago by
Same here. For me, it happened the first time I tried to remove something from the "blocked by"/"blocking" fields.
My main problem is that even after I removed all values from the "blocked by" and "blocking" fields from the related tickets, the depgraph is still the same. The content of the graph and the content of the ticket remained inconsistent, even after I restarted Trac.
System Information
Trac 1.0 Genshi 0.6 (without speedups) pysqlite 2.6.0 Python 2.7.3 (default, Apr 10 2012, 23:31:26) [MSC v.1500 32 bit (Intel)] setuptools 0.6c11 SQLite 3.6.21 jQuery 1.7.2
Installed Plugins
tracaccountmanager 0.3.2 c:\python27\lib\site-packages\tracaccountmanager-0.3.2-py2.7.egg tracmastertickets 3.0.2 d:\swankey-server\trac-environment\ammo\plugins\tracmastertickets-3.0.2-py2.7.egg
comment:3 Changed 12 years ago by
Occurs here too after removing "Blocking" tickets.
Note that the blocking tickets I had added were already closed (it might help..).
Traceback:
Most recent call last: File "/usr/lib/python2.6/dist-packages/trac/web/main.py", line 511, in _dispatch_request dispatcher.dispatch(req) File "/usr/lib/python2.6/dist-packages/trac/web/main.py", line 237, in dispatch resp = chosen_handler.process_request(req) File "/usr/lib/python2.6/dist-packages/trac/ticket/web_ui.py", line 169, in process_request return self._process_ticket_request(req) File "/usr/lib/python2.6/dist-packages/trac/ticket/web_ui.py", line 541, in _process_ticket_request self._do_save(req, ticket, action) File "/usr/lib/python2.6/dist-packages/trac/ticket/web_ui.py", line 1238, in _do_save cnum=internal_cnum): File "/usr/lib/python2.6/dist-packages/trac/ticket/model.py", line 360, in save_changes listener.ticket_changed(self, comment, author, old_values) File "/usr/lib/pymodules/python2.6/mastertickets/api.py", line 111, in ticket_changed links.save(author, comment, tkt.time_changed, db) File "/usr/lib/pymodules/python2.6/mastertickets/model.py", line 68, in save update_field(new_value) File "/usr/lib/pymodules/python2.6/mastertickets/model.py", line 61, in <lambda> update_field = lambda lst: lst.remove(str(self.tkt.id))
Enabled plugins:
acunu-commit-updater N/A /srv/trac/plugins/acunu_commit_updater.pyc BatchModify 0.8.0-trac0.12 /usr/lib/pymodules/python2.6 ticket-clone N/A /srv/trac/plugins/ticket_clone.pyc TracMasterTickets 3.0.2 /usr/lib/pymodules/python2.6 TracMercurial 0.12.0.28dev /usr/lib/pymodules/python2.6 TracProgressMeterMacro 0.3 /usr/lib/pymodules/python2.6
comment:4 Changed 12 years ago by
Owner: | changed from Noah Kantrowitz to Ryan J Ollos |
---|---|
Priority: | normal → high |
Status: | new → assigned |
comment:5 Changed 12 years ago by
I get the same error when I try to change a ticket from "blocked by" to "blocking."
Python Traceback
Most recent call last: File "/opt/kforge19/lib/python2.6/site-packages/trac/web/main.py", line 511, in _dispatch_request dispatcher.dispatch(req) File "/opt/kforge19/lib/python2.6/site-packages/trac/web/main.py", line 237, in dispatch resp = chosen_handler.process_request(req) File "/opt/kforge19/lib/python2.6/site-packages/trac/ticket/web_ui.py", line 169, in process_request return self._process_ticket_request(req) File "/opt/kforge19/lib/python2.6/site-packages/trac/ticket/web_ui.py", line 541, in _process_ticket_request self._do_save(req, ticket, action) File "/opt/kforge19/lib/python2.6/site-packages/trac/ticket/web_ui.py", line 1238, in _do_save cnum=internal_cnum): File "/opt/kforge19/lib/python2.6/site-packages/trac/ticket/model.py", line 360, in save_changes listener.ticket_changed(self, comment, author, old_values) File "build/bdist.linux-x86_64/egg/mastertickets/api.py", line 111, in ticket_changed links.save(author, comment, tkt.time_changed, db) File "build/bdist.linux-x86_64/egg/mastertickets/model.py", line 68, in save update_field(new_value) File "build/bdist.linux-x86_64/egg/mastertickets/model.py", line 61, in <lambda> update_field = lambda lst: lst.remove(str(self.tkt.id))
Enabled Plugins
EstimationTools 0.4.6-r12062 /opt/kforge_project/gripperdesign/trac/695/plugins/EstimationTools-0.4.6_r12062-py2.6.egg timingandestimationplugin 1.2.8 /opt/kforge_project/gripperdesign/trac/695/plugins/timingandestimationplugin-1.2.8-py2.6.egg Trac-jsGantt 0.10-r11863 /opt/kforge_project/gripperdesign/trac/695/plugins/Trac_jsGantt-0.10_r11863-py2.6.egg TracGit 0.12.0.2dev-r7757 /opt/kforge19/lib/python2.6/site-packages TracMasterTickets 3.0.2 /opt/kforge_project/gripperdesign/trac/695/plugins/TracMasterTickets-3.0.2-py2.6.egg TracMercurial 0.12.0.28dev-r10698 /opt/kforge19/lib/python2.6/site-packages TracSubTicketsPlugin 0.1.0 /opt/kforge_project/gripperdesign/trac/695/plugins/TracSubTicketsPlugin-0.1.0-py2.6.egg
comment:6 Changed 11 years ago by
This will fix it for clean installs. if you have already used the currently broken version, your data is in an invalid state and you will have to fix it manually in the database..you'll have to add the missing ticket_custom entries for referenced tickets.. sucks, i know.. might be easier to start over.. anyway the problem is that the code was trying to fall back to an insert if the update failed, but someone shoehorned unrelated code in between the two queries, breaking the insert.
-
home/treksler/masterticketsplugin/trunk/mastertickets/model.py
61 61 update_field = None 62 62 if n in new_ids and n not in old_ids: 63 63 # New ticket added 64 cursor.execute('INSERT INTO mastertickets (%s, %s) VALUES (%%s, %%s)' % sourcedest, 65 (self.tkt.id, n)) 64 cursor.execute('INSERT INTO mastertickets (%s, %s) VALUES (%%s, %%s)' % sourcedest, (self.tkt.id, n)) 66 65 update_field = lambda lst: lst.append(str(self.tkt.id)) 67 66 elif n not in new_ids and n in old_ids: 68 67 # Old ticket removed … … 70 69 update_field = lambda lst: lst.remove(str(self.tkt.id)) 71 70 72 71 if update_field is not None: 73 cursor.execute('SELECT value FROM ticket_custom WHERE ticket=%s AND name=%s', 74 (n, str(field))) 72 cursor.execute('SELECT value FROM ticket_custom WHERE ticket=%s AND name=%s', (n, str(field))) 75 73 old_value = (cursor.fetchone() or ('',))[0] 76 74 new_value = [x.strip() for x in old_value.split(',') if x.strip()] 77 75 update_field(new_value) … … 103 101 'INSERT INTO ticket_change (ticket, time, author, field, oldvalue, newvalue) VALUES (%s, %s, %s, %s, %s, %s)', 104 102 (n, when_ts, author, 'comment', '', '(In #%s) %s' % (self.tkt.id, comment))) 105 103 106 cursor.execute('UPDATE ticket_custom SET value=%s WHERE ticket=%s AND name=%s', 107 (new_value, n, field)) 104 cursor.execute('UPDATE ticket_custom SET value=%s WHERE ticket=%s AND name=%s', (new_value, n, field)) 105 if not cursor.rowcount: 106 cursor.execute('INSERT INTO ticket_custom (ticket, name, value) VALUES (%s, %s, %s)', (n, field, new_value)) 108 107 109 108 # refresh the changetime to prevent concurrent edits 110 109 cursor.execute('UPDATE ticket SET changetime=%s WHERE id=%s', (when_ts, n)) 111 110 112 if not cursor.rowcount:113 cursor.execute('INSERT INTO ticket_custom (ticket, name, value) VALUES (%s, %s, %s)',114 (n, field, new_value))115 116 111 # cursor.execute('DELETE FROM mastertickets WHERE source=%s OR dest=%s', (self.tkt.id, self.tkt.id)) 117 112 # data = [] 118 113 # for tkt in self.blocking:
comment:7 Changed 11 years ago by
As mentioned in my previous comment, the cause of this ticket is code that was inserted in the wrong place. This code was inserted in [12912] The above patch changes the order to be correct. Cheers
comment:8 Changed 11 years ago by
Priority: | high → highest |
---|---|
Severity: | normal → blocker |
elevating to blocker, because this ticket completely breaks the plugin by not updating referenced tickets properly, leading to manual database intervention.
comment:9 Changed 11 years ago by
Summary: | ValueError: list.remove(x): x not in list → [patch] ValueError: list.remove(x): x not in list |
---|
comment:11 follow-up: 19 Changed 11 years ago by
I'm not sure when I'll have time to do it, but hopefully within the next month. I'm not willing to apply the patch outright without reviewing and testing. The plugin is in pretty bad shape because of all the monkey-patching over the past several years. If you have a change to test out the patch and can report back, that could speed up the process.
comment:12 follow-up: 15 Changed 11 years ago by
That sounds like a good idea, though its not a high priority on my end. I see that there aren't any existing automated tests as well...
comment:13 Changed 11 years ago by
Unfortunately, I haven't been able to reproduce the errors using trac normally. It would be great if there was a reliable way to trigger the errors.
I have written tests for the patch though which can be found at [0] by mocking out the db and faking db execution results.
[0] https://github.com/milki/trac-mastertickets/commits/milki-ensure-ticket-update-synced-8792
comment:14 Changed 11 years ago by
Same problem for me using Trac 0.12.2 stable and TracMasterTickets 3.0.2
comment:15 Changed 11 years ago by
Replying to milki:
That sounds like a good idea, though its not a high priority on my end. I see that there aren't any existing automated tests as well...
not a high priority? the patch is trivial and obvious, and without it people are ruining their databases. doesn't get any higher priority than that...
comment:16 Changed 11 years ago by
Cc: | mwisnicki@… added |
---|
comment:17 Changed 11 years ago by
Cc: | rockethacker added |
---|
comment:19 Changed 9 years ago by
Replying to rjollos:
I'm not sure when I'll have time to do it, but hopefully within the next month.
In the mean time you could write something like "DO NOT USE - RUINS YOUR DATABASE!" above all download links. Or, better: make current download links break and make nwe ones in a section "broken dangerous code".
comment:20 Changed 9 years ago by
One delete statement helped in my case as a workaround:
DELETE FROM trac.mastertickets WHERE dest = 30077;
- 30077 was the ticket having the issue.
- I also moved the two lines of codes as in the patch in comment:6 before I deleted. No clue whether that made a difference though.
To analyse the situation the following helped for ticket 30077:
SELECT * FROM trac.mastertickets WHERE source = 30077; SELECT * FROM trac.mastertickets WHERE dest = 30077; SELECT * FROM trac.ticket_custom where ticket = 30077;
comment:21 Changed 9 years ago by
Why database transactions are not used to avoid database inconsistency?
comment:22 Changed 9 years ago by
The plugin needs to be adapted to the Trac 1.0 database API. See #12193. trac:PatchWelcome.
comment:23 Changed 8 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Please create a new issue if the behavior is seen after #12193.