Merge lp:~therp-nl/openupgrade-server/6.0-lp1025797-dangling_references into lp:openupgrade-server/6.0

Proposed by Stefan Rijnhart (Opener) on 2012-07-18
Status: Merged
Merged at revision: 3493
Proposed branch: lp:~therp-nl/openupgrade-server/6.0-lp1025797-dangling_references
Merge into: lp:openupgrade-server/6.0
Diff against target: 23 lines (+13/-0)
1 file modified
bin/osv/orm.py (+13/-0)
To merge this branch: bzr merge lp:~therp-nl/openupgrade-server/6.0-lp1025797-dangling_references
Reviewer Review Type Date Requested Status
Don Kirkby (community) 2012-07-18 Disapprove on 2012-07-18
Holger Brunn (Therp) 2012-07-18 Approve on 2012-07-18
Review via email: mp+115518@code.launchpad.net

Description of the change

As reported in the associated issue, OpenERP 5 leaves dangling references to removed resources in ir_model_data. This branch checks for them at model initialization time and removes them.

To post a comment you must log in.
review: Approve
Don Kirkby (donkirkby) wrote :

This proposal would solve the error, as well as bug 1025819, along with any other modules where deleted records cause problems.

However, I don't like the fact that records magically reappear during the migration, perhaps months after some administrator deleted them. That's why I recreated them as inactive records in my merge proposal.

How about a third option? Create a working table and copy all the dangling ir_model_data records there, then delete them. At the end of the migration, go through the working table and deactivate all the records that got recreated. That way, it's a general solution that doesn't require custom scripting for each module, you don't have to make up bogus values for the recreated records, but the deleted records still aren't visible.

If you agree in principle, I'll make a merge proposal for you to review. If you still prefer the current proposal, I can work around it by recreating the records before I start the migration.

review: Disapprove

Hi Don,

thank you for your opinion. I understand you really want to see lp:1023615 fixed but it must be fixed in the OpenERP server, not in this project.

Stefan.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/osv/orm.py'
2--- bin/osv/orm.py 2012-05-27 12:34:50 +0000
3+++ bin/osv/orm.py 2012-07-18 10:49:22 +0000
4@@ -2739,6 +2739,19 @@
5 for order, f, k in todo_update_store:
6 todo_end.append((order, self._update_store, (f, k)))
7
8+ # OpenUpgrade: remove dangling references from ir_model_data
9+ cr.execute("""
10+ DELETE FROM ir_model_data
11+ WHERE model = \'%s\'
12+ AND res_id not in (
13+ SELECT id FROM %s
14+ )
15+ """ % (self._name, self._table))
16+ if cr.rowcount:
17+ self.__logger.debug(
18+ "Removed %s dangling reference(s) to resources of model %s "
19+ "in ir_model_data", cr.rowcount, self._name)
20+
21 else:
22 cr.execute("SELECT relname FROM pg_class WHERE relkind IN ('r','v') AND relname=%s", (self._table,))
23 create = not bool(cr.fetchone())

Subscribers

People subscribed via source and target branches