On 12-09-19 10:52 AM, Barry Warsaw wrote: >> Yes, I know. It's not clear to me exactly how this will fit into >> TestDBus though, because SharedModel is not like our other DBus classes >> where we are defining the classes ourselves and have a lot of control >> over them. I have no idea if SharedModel will respect the custom >> sessionbus that TestDBus creates, or how to ensure that that is >> happening. I was hoping you'd offer a bit more guidance in this aspect. > > I don't know a lot about SharedModel yet, but Ken mentioned on IRC that using > a temporary session bus will isolate the model changes. This should be fairly > easy to verify, though the minimal documentation I've found on Dee doesn't > really say. I'd be more worried about any caching polluting the user's > environment. We'll just have to hack on this to make sure. The testsuite, as it currently stands in the mp, does not pollute the users environment. The tests that test Base._publish are using a mocked SharedModel using a different bus name, and there's even a test that confirms the user's environ is not polluted. The other test, that tests SharedModel more directly, does operate on the user's live environment, but it's a read-only test that doesn't do any polluting. It basically just tests that the right schema is in place, which is probably good enough, because anything more than that would become a test against Dee, rather than a test against Gwibber. >> Is it done in such a way that importing SharedModel will force it to use the >> custom bus and not the real bus? > > Only in the gwibber-service executed by dbus-daemon described above. > >> Because SharedModel handles it's own dbus logic internally, I don't see any >> way to tell SharedModel 'hey, use this other bus instead' unless your >> TestDBus magic is somehow affecting the dbus module itself in a >> process-global way. > > Yep, it is. `M-x man RET dbus-daemon RET` will provide additional > information. Essentially, it's dbus-daemon that's doing all the magic that > the dbus library needs to run gwibber-service on an isolated session bus. Ok, so importing our SharedModel instance in main.py would result in an isolated, testable SharedModel. good to know. > We can keep the regexp. Funny, because I actually did replace it with a string comprehension, and the readability did improve incredibly. My performance concerns were mitigated by a) no longer iterating over the sharedmodel, calling strip() each time, and b) caching the results of strip(), such that it's only necessary to call it once for each new message addition. That effectively upgrades it from O(n) to O(1), which means the slower implementation won't really be noticable. >> It turns out that concatenating the sender's name and message text >> *only* is the correct approach for fuzzy equivalence testing ;-) > > No, because it's just that strip() or whatever would pull out row[sender] and > row[message] and do the concatenation itself. It just moves that logic into > the method that calculates the unique key from the row data. Oh, now i see. You can make that change if you insist ;-) >>> Also, is there no more efficient way of asking whether the key already >>> exists in the database than iterating through all the rows? Couldn't that >>> get extremely inefficient? >> >> That's a good question, I'm not sure. The problem is that >> Dee.SharedModel is kind of a custom gobjecty thing, and it's not well >> documented, and I'm not sure how good the python overrides are (it has >> some but seemingly not all of the methods you would expect to be able to >> treat it as a true iterable). > > The best I could find is this: > > http://developer.ubuntu.com/api/ubuntu-12.04/python/Dee-1.0.html > > SharedModel implements this: > > http://developer.ubuntu.com/api/ubuntu-12.04/python/Dee-1.0.html#Dee.Model > > and yeah, the documentation is, um, sparse. So it's probably worth playing > around with this to see if there's a better way to find the row. > find_row_sorted() looks promising, as does Dee.Index.lookup(). You gotta > figure there's an efficient way to query the table. Yeah, I already had good results from using a module-global dict to map pre-stripped strings -> DeeModelIters, meaning we no longer need to iterate over the model every time a message is added. >>>> + return bool(SharedModel.append(*args)) >>> >>> What does SharedModel.append() actually return? >> >> A DeeModelIter, which can be used to modify the row later. Very much >> like a GtkListStore if you're familiar with those. I'm not actually >> aware of what might cause .append to fail, but it just seemed like the >> smart thing to do to be able to report a failure as False, but then >> again it's not actually clear to me that any other code is going to >> check or care about the return value from Base._publish. > > I guess my question is, what is the truth value of a DeeModelIter? There's no > documentation on it, or whether .append() can return None or something else if > it fails. Either we'll have to do some experiments to find out what it does > in practice, or maybe just kill the return value. Well, it's a python object, and thus it's always true ;-) I've changed the return value to be more indicative of whether or not the message was successfully added, and I think you'll like it. Simple, one line, clear meaning. Please merge ;-)