From: Michiel N. <m.d...@he...> - 2011-02-22 19:08:26
|
Hi Nick, If the gtk code is asynchronous why would it then be only the listviews that give trouble, why not also gramplets for example? Isn't the right solution to emit anti signals from transaction_abort. So if in the transaction something is added (TXNADD) to sent an -delete signal, and if something is deleted (TXNDEL) to send the the -add signal? This would also increase the symmetry between transaction_commit and transaction_abort. Problem is that most code can't cope with deleting things that are not yet there, or adding things that are already there. The delete_row_by_handle in flatbasemodel.py seems to be to positive exception with the "is self.node_map.get_path(handle) is None: return" at the beginning. Michiel On 02/22/2011 05:35 PM, Nick Hall wrote: > > > Benny Malengier wrote: >> >> >> 2011/2/21 Nick Hall <nic...@ho... >> <mailto:nic...@ho...>> >> >> >> >> Benny Malengier wrote: >> >> >> >> 2011/2/21 Nick Hall <nic...@ho... >> <mailto:nic...@ho...> <mailto:nic...@ho... >> <mailto:nic...@ho...>>> >> >> >> >> Benny Malengier wrote: >> >> >> >> 2011/2/21 Nick Hall <nic...@ho... >> <mailto:nic...@ho...> >> <mailto:nic...@ho... >> <mailto:nic...@ho...>> >> <mailto:nic...@ho... <mailto:nic...@ho...> >> >> <mailto:nic...@ho... >> <mailto:nic...@ho...>>>> >> >> >> Michiel, >> >> When I was updating the tagging code, I noticed that the >> object-delete >> signals are still issued after the object has been >> deleted. >> >> The tagging code tries to get the deleted tag from the >> handle, which >> causes a problem. I can get around this, but it could >> cause a problem >> elsewhere. >> >> Should I modify the tagging code to cope with this >> change, or >> should we >> issue the object-delete signal before the object is >> removed >> from the >> database? >> >> >> No, you should change your code. >> If you need data from objects after deletion (other >> than the >> passed handle), you should cache that info. >> The reason I agreed with this change of Michael is that >> deleting can take a long time if we wait for end of signal >> before doing the delete. >> This is problematic, as the window for a crash, and >> hence an >> abort of the transaction, increases dramatically like this. >> Doing delete signal after the actual delete means we don't >> loose the transaction if the code reacting to the >> signal crashes. >> You see this in gen/db/write.py, transaction_commit method >> does the self.bsddbtxn.commit() code and then signals are >> triggered, so crash of those no longer undoes the bsddb >> change. >> >> >> I just wanted to check that this change was intentional. >> >> I have committed some updated tagging code (r16690). >> >> >> >> >> Another reason is that a rebuild signal also happens >> after a >> batch transaction, so your code should already be able to >> handle that. >> >> >> Yes, I already handle the rebuild signal, but I re-read the >> whole >> table when I receive this. As the tags table is likely to be >> small, I suppose I could save a few lines of code by doing a >> rebuild for all changes, but it is not necessary. >> >> There are a couple of issues that I noticed when testing the >> tagging code. >> >> 1. When a transaction is aborted it still appears in the undo >> history. >> >> >> Post a bug for this. >> >> >> >> Done. >> >> http://www.gramps-project.org/bugs/view.php?id=4665 >> >> >> >> >> 2. Long transactions cause unwanted effects in the listviews. >> This is because the model changes when the database is >> updated, >> but the signals can take some time to arrive. >> >> >> Hmm, I thought the model only updates on the signals. >> >> >> Because the model retrieves data from the database, it is >> effectively updated when changes are made to the database. When >> the model changes it should send a signal to the TreeView so that >> it can be kept synchronised with the data. >> >> > > I have created another bug for this. > > http://www.gramps-project.org/bugs/view.php?id=4669 > > >> These are triggered columns, not much we can do. Only when you move >> over the column the data is obtained, so the model updates the moment >> you move over it. There are no signals involved. That is why the cache >> on column data should cover the normal occurrence of this. > > The problem is the delay in receiving a signal. The gtk implementation > of treeviews is in two parts: the TreeView handles the GUI and the model > stores the data. Under normal circumstances, when data changes in the > model it should emit a signal to tell the GUI that something has > changed. When using the gtk provided models, all of the signals are > managed automatically for you. They are also emitted quickly after the > data is changed. > > >> Normal action is that there is no abort, so things should end up >> correct. To me it seems the main problem is that changing tags can >> take a long time, but we want to be able to undo it. >> >> >> >> So you mean code that fetches from database while the data is >> no longer present so as to fill rows? >> >> >> It looks like the TreeView is fetching updated data from the model >> before it receives the appropriate signal. >> >> >> Yes, this is normal behavior. We indicated in the bug ticket that >> async GTK would be the one thing that can give issues with the changed >> behavior. >> >> Any specific use-case? >> >> >> I only noticed it when I was testing the changes I made to the >> tagging code. I haven't investigated it further yet. >> >> Enable the tags column on the flat person view, then try adding >> and removing tags to large numbers of people. Moving the mouse >> cursor over the TreeView will change the contents of the tags column. >> >> Try aborting adding a tag to all people in the database. The view >> will display the new tags even when the process is aborted. >> Moving the cursor over the TreeView or clicking on a row will >> update the display. This doesn't look good. >> >> >> This can be fixed with calling the rebuild signals on abort. > > I don't think that we need a complete rebuild. I could just send the > appropriate signals to the TreeView, as I do when a tag colour is changed. > > >> The current Gramps code has the same problem, it is just more >> difficult to trigger, as all change calls happen consecutively at the >> end of the commit. This does give the idea that perhaps your update >> code is doing too much in between changes of objects. > > I have looked at my code for adding and removing tags. It does about the > absolute minimum at the moment. Apart from updating the progress bar, in > the loop it gets the object from the tag, adds or removes and tag from > it and then commits the change. > > > Nick. > > >> >> As you know this update will take a lot of time, you might make the >> main Gramps window blocked so a user cannot interact with it. >> >> Benny >> >> >> I could disable the cancel button on the progress bar. (I noticed >> that the cancel button on the progress bar when removing tags >> doesn't work. I'll look into this if we decide to keep this >> functionality.) >> >> >> Nick. >> >> >> >> I was hoping the cache in the listviews would handle this >> sufficiently (so only hitting the model, not the changed >> database). >> Benny >> >> > |