Code review comment for lp:~itohm/nova/lp735974

Revision history for this message
Jay Pipes (jaypipes) wrote :

Hi!

The synchronize_session parameter is good, thanks!

However, it looks like that update should also be setting the updated_at column as well, no?

So, could you add it, like so:

update('instance_id': None,
       'leased': 0,
       'updated_at', datetime.datetime.utcnow()}, synchronize_session='fetch')

When running update() form a Query object in SQLalchemy, the normal before_update() hooks that would normally set updated_at automatically are not fired, which is why I believe the manual setting of updated_at is necessary.

Cheers!
jay

review: Needs Fixing

« Back to merge proposal