Code review comment for lp:~sleepsonthefloor/nova/709057

Revision history for this message
Devin Carlen (devcamcar) wrote :

5 with session.begin():
6 instance_ref = instance_get(context, instance_id, session=session)
7 instance_ref.delete(session=session)
8 + session.execute('update security_group_instance_association'
9 + ' set deleted=1 where instance_id=:id',
10 + {'id': instance_id})

Would prefer to not see mixing of the sqlalchemy orm and raw sql code in a single operation. Eventually we should replace the raw sql with the sqlalchemy query language (which is essentially in between the orm and raw sql, a rather happy medium).

I propose making lines 6 and 7 match this style:

16 session.execute('update security_groups set deleted=1 where id=:id',
17 {'id': security_group_id})

This also has the advantage of not eating an extra query looking up the instance_ref, which instance_ref.delete currently does.

review: Needs Fixing

« Back to merge proposal