launchpad-reviewers team mailing list archive

[Guilherme Salgado <guilherme.salgado@xxxxxxxxxx>]

Oh, so I was wrong all along, thinking the event was what we were missing, and you didn't tell me? ;)

So, now that you pointed me in the right direction I can see where this is coming from... The event is fired by lazr.restful whenever someone calls a remote operation (like updateStatus(), which is exported as a write operation), so that's why you don't get the event when you call updateStatus() directly. I suggest you add a comment to the test explaining why you need to fire the event yourself and that in production this is not needed.  However, if someone edits the workitems using the +workitems page, no notification will be sent because that code doesn't call updateStatus() via lazr.restful and so no event is fired. We might need to have that page itself fire an ObjectModifiedEvent...

Now, it looks like the work item changes are not always flushed to the DB when notify_specification_modified is fired (as the pdb session below shows), so that would explain why the diff looks wrong in some cases.  

(this is within notify_specification_modified)
(Pdb) p spec.workitems_text
''
(Pdb) !from lp.services.database.sqlbase import flush_database_updates
(Pdb) p flush_database_updates()
None
(Pdb) p spec.workitems_text
u'Work items:\nfoo: TODO\nbar: DONE'

I wouldn't expect this tot happen because spec.workitems_text will do a DB query and before executing that storm will flush the updates to the DB.  As you've found out, calling transaction.commit() solves that, but that is not something we should be doing (we should commit only at the end of the request processing routine, if everything succeeds).  A better way to workaround this is to call flush_database_updates(), probably in updateWorkItems(), but first I'll try to figure out why we need that so that we can at least leave a comment explaining it.
-- 
https://code.launchpad.net/~linaro-infrastructure/launchpad/notify-workitems-changes/+merge/98231
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~linaro-infrastructure/launchpad/notify-workitems-changes into lp:launchpad.