| 00:01.05 | *** join/#storm wallflower (n=wallflow@ip-205-246-113-216.pool.grokthis.net) |
| 01:10.56 | *** join/#storm niemeyer_ (n=niemeyer@200-138-34-156.ctame705.dsl.brasiltelecom.net.br) |
| 01:19.15 | *** join/#storm jbot (i=ibot@pdpc/supporter/active/TimRiker/bot/apt) |
| 01:19.15 | *** topic/#storm is The Storm Python ORM - http://storm.canonical.com/ - 0.13 released! || Review branches: https://code.launchpad.net/~storm/storm/trunk/+landing-candidates |
| 02:47.30 | *** join/#storm mwhudson (n=mwh@canonical/launchpad/mwhudson) |
| 03:30.45 | *** join/#storm jamesh (n=james@canonical/launchpad/jamesh) |
| 05:38.24 | *** join/#storm jml (n=jml@mumak.net) |
| 05:56.34 | *** join/#storm jukart (n=jukart@lsfw01.lovelysystems.com) |
| 06:03.49 | *** join/#storm dobee (n=dobee@lsfw01.lovelysystems.com) |
| 06:22.24 | *** join/#storm jukart (n=jukart@lsfw01.lovelysystems.com) |
| 07:13.18 | *** join/#storm goschtl (n=goschtl@p5B0BD1B5.dip.t-dialin.net) |
| 07:24.33 | *** join/#storm dobee (n=dobee@lsfw01.lovelysystems.com) |
| 08:24.02 | *** join/#storm mwhudson (n=mwh@canonical/launchpad/mwhudson) |
| 08:50.59 | *** join/#storm lightyear (n=ben@nat/fluendo/x-4c2a3c8ee0d104c6) |
| 10:13.24 | jamesh | therve: there is a test failure with your latest merge to storm's trunk |
| 10:13.33 | jamesh | I've filed https://bugs.launchpad.net/storm/+bug/276690 with the details. |
| 10:13.40 | therve | oops |
| 10:13.45 | therve | jamesh: thanks, I'll look at it |
| 10:14.31 | therve | hum, it passes for me |
| 10:15.17 | jamesh | the __hash__() implementation for all variable classes is insane. |
| 10:16.59 | therve | what's your python version? |
| 10:17.15 | jamesh | 2.5.2 |
| 10:17.32 | jamesh | the standard build on hardy/amd64 |
| 10:17.52 | therve | ah 64 bits, interesting |
| 10:18.22 | jamesh | having hash() implemented on an object that can mutate its value breaks dictionaries and sets |
| 10:19.05 | jamesh | the hacks in PickleVariable.__hash__ and ListVariable.__hash__ should have been warning signals |
| 10:19.35 | jamesh | (those bits aren't from your branch -- your branch just tickled the existing issue) |
| 10:20.23 | therve | right |
| 10:22.59 | jamesh | there seems to be a lot of code that depends on variables being hashable |
| 10:23.12 | jamesh | and hashable based on value |
| 10:25.31 | therve | I don't understand why it doesn't fail for me though |
| 10:27.22 | jamesh | me either. |
| 10:27.33 | jamesh | actually, yes I do. |
| 10:28.48 | therve | ah? |
| 10:29.46 | jamesh | so, both variables are hooked to the start-tracking-changes event |
| 10:30.09 | jamesh | the order they get called depends on set iteration order |
| 10:30.48 | jamesh | if prop1 gets called first, then its _detect_changes() handler gets hooked to the "flush" event |
| 10:31.08 | jamesh | if they get called in the other order, then prop2's _detect_changes() wins |
| 10:31.15 | therve | ahah right |
| 10:31.27 | jamesh | having different set iteration order on different platforms is not too surprising |
| 10:32.23 | therve | indeed |
| 10:38.15 | *** join/#storm jamesh_ (n=james@canonical/launchpad/jamesh) |
| 10:43.02 | therve | jamesh_: what do you thin of that: http://paste.ubuntu.com/52792/ |
| 10:43.41 | therve | I think it's makes sense to special case these variables, as there are mutable ones |
| 11:06.02 | *** join/#storm niemeyer (n=niemeyer@200-138-34-156.ctame705.dsl.brasiltelecom.net.br) |
| 11:06.57 | jamesh | therve: I tried something like that, and it seemed to pass the tests |
| 11:07.24 | jamesh | therve: but having hash() implemented for _any_ variable is wrong when combined with the __eq__() implementation |
| 11:09.03 | jamesh | niemeyer: when you've got time, could you give some input on https://bugs.launchpad.net/storm/+bug/276690 ? |
| 11:09.18 | niemeyer | jamesh: Hey! |
| 11:09.20 | niemeyer | jamesh: Of course |
| 11:09.30 | niemeyer | jamesh: I'll check it out right now |
| 11:12.50 | jamesh | I wonder if some variant of "frozen" variables is needed. |
| 11:13.41 | jamesh | hmm. That wouldn't really help for some of the other uses :( |
| 11:13.58 | niemeyer | jamesh: Interesting |
| 11:14.49 | niemeyer | jamesh: I'm not entirely sure if that's a problem right now |
| 11:14.58 | niemeyer | jamesh: I mean, we have to fix this behavior for sure |
| 11:15.17 | niemeyer | jamesh: But the use case for hashing variables now doesn't mutate them |
| 11:15.40 | niemeyer | jamesh: At least I think so, from my vague memories |
| 11:15.54 | jamesh | niemeyer: adding a variable method as an event handler hashes the variable. |
| 11:16.29 | niemeyer | jamesh: Right, that was introduced with the recent change |
| 11:16.30 | jamesh | or even including a variable as an extra argument to an event handler will |
| 11:16.58 | niemeyer | jamesh: I wasn't clear, sorry. I was just wondering if we have a bug in the last release or not |
| 11:17.21 | *** join/#storm andrea-bs (n=andrea@ubuntu/member/beeseek.developer.andrea-bs) |
| 11:17.55 | niemeyer | jamesh: We should definitely do what you suggest and hash by identity |
| 11:18.08 | jamesh | niemeyer: PickleVariable.__init__() sure looks like it is hooking methods before therve's merge. |
| 11:18.32 | niemeyer | jamesh: The use case for the current hashing is the caching of objects in the Store.. it should be trivial to fix that with different logic |
| 11:18.33 | jamesh | so that aspect isn't new. |
| 11:18.47 | jamesh | yeah. The store._alive dictionary |
| 11:19.18 | niemeyer | jamesh: Ok, so we have to do a 0.14 soon |
| 11:21.01 | jamesh | I guess no one was using >1 Pickle or List columns in a single class enough to notice ... |
| 11:21.58 | niemeyer | jamesh: Yeah |
| 11:24.33 | niemeyer | jamesh,therve: I wonder if using the value of get(to_db=True) as the alive key is semantically correct |
| 11:52.53 | jukart | niemeyer: just run into a problem : if a column for a select is a Select it is not surrounded by parenthesis :( |
| 11:55.25 | therve | niemeyer: humm, where does that come from? |
| 11:55.58 | niemeyer | jukart: Can you please file a bug about this one? Should be easy to fix |
| 11:56.13 | niemeyer | therve: From the variable (?) |
| 11:59.46 | therve | niemeyer: sorry, I'm not following you |
| 11:59.53 | therve | niemeyer: what's the alive key? |
| 12:00.05 | niemeyer | therve: The key of the alive dictionary in the Store |
| 12:00.40 | therve | so for me it's (cls_info.cls, tuple(primary_vars)) ? |
| 12:02.26 | niemeyer | therve: Right |
| 12:02.51 | niemeyer | therve: That's the only reason why variable hashes are as they are now |
| 12:03.06 | niemeyer | therve: We can easily fix the issues we have by replacing the keying |
| 12:05.03 | therve | ok I may get it |
| 12:06.05 | therve | right now we're using the variable *object* into the alive dict |
| 12:06.13 | therve | instead of using the variable value |
| 12:06.49 | therve | thus leading to your comment on get(to_db=True) |
| 12:07.08 | therve | (phew, I catch up) |
| 12:07.31 | niemeyer | therve: That's it! :-) |
| 12:07.38 | therve | :) |
| 12:08.07 | therve | so yeah, it looks good semantically |
| 12:08.11 | jamesh | niemeyer: well, they also need to be hashable to be used in event handlers. |
| 12:08.40 | niemeyer | jamesh: How do you mean? |
| 12:08.43 | therve | jamesh: but if we fix the problem in the alive dict, we can remove uoru custom __hash__ |
| 12:08.49 | niemeyer | Right |
| 12:08.56 | therve | s/uoru/our |
| 12:09.06 | jamesh | niemeyer: the hooked callable + extra arguments are stored in a set, so need to be hashable |
| 12:09.30 | niemeyer | jamesh: Right, these expect the variable to be hashable by identity |
| 12:09.48 | niemeyer | jamesh: If we change the behavior of the alive key, we can remove the custom hashing |
| 12:10.18 | jamesh | niemeyer: having hash() go by identity but == go by value is still a problem |
| 12:10.33 | therve | that's true... |
| 12:10.43 | therve | do we need __eq__ ? |
| 12:10.44 | jamesh | if two items end up in the same hash bucket, then you've still got the same problem |
| 12:12.27 | jamesh | there were a bunch of tests that failed when I commented it out, but I didn't check to see if they were all related to store._alive |
| 12:13.03 | jamesh | you really do need "A == B" to imply "hash(A) == hash(B)" |
| 12:13.05 | niemeyer | jamesh: Yeah, we'd have to remove it too.. I imagined that the only reason why that equality is in place is the hashing of alive too |
| 12:13.07 | jamesh | or things go wobbly |
| 12:13.13 | niemeyer | jamesh: But it's been two years, so my memory is fading |
| 12:23.33 | jukart | niemeyer: reported here : https://bugs.launchpad.net/storm/+bug/276741 |
| 12:23.40 | niemeyer | jukart: Thanks! |
| 12:24.05 | jukart | niemeyer: my problem is now, I can't find a workaround :( |
| 12:24.18 | johan | jamesh: are you working on improving django support? |
| 12:24.38 | johan | eg, admin integration |
| 12:24.45 | niemeyer | jukart: Try to use an Alias perhaps |
| 12:24.54 | niemeyer | johan: Morning! |
| 12:24.59 | johan | hey niemeyer |
| 12:25.15 | johan | niemeyer: I started to move Stoq over to storm, having plenty of fun :-) |
| 12:26.30 | niemeyer | johan: Yeah, I can imagine that :) |
| 12:26.43 | jukart | niemeyer: work perfect, thanks for the hint |
| 12:26.51 | niemeyer | jukart: Sweet! |
| 12:28.40 | therve | so removing eq/hash, and fixing alive to use get(to_db=True), seems to work |
| 12:28.57 | therve | 52 test failures, mostly from test comparing 2 Variables instances |
| 12:29.08 | jukart | niemeyer: added the workaround to the bug description |
| 12:29.14 | therve | I'm a bit worried about the performance impact though |
| 12:35.05 | niemeyer | therve: It may be a bit more costy indeed |
| 12:35.17 | niemeyer | therve: But there are some savings on equality comparisons, for instance |
| 12:35.52 | niemeyer | therve: Have you made any rough tests to have an idea of the impact? |
| 12:36.06 | therve | niemeyer: not really |
| 12:36.18 | therve | niemeyer: I can try to run landscape test suite :) |
| 12:37.01 | niemeyer | therve: Cool, that might be nice.. also that test you made in the sprint which had a large number of objects alive might give a hint |
| 12:37.49 | therve | oh good idea |
| 12:43.31 | *** join/#storm kov (n=kov@debian/developer/kov) |
| 14:03.15 | jamesh | johan: integrating with the django admin interface is not something I plan to spend time on. |
| 14:03.35 | johan | jamesh: oh, too bad. It would have been a killer feature. |
| 14:03.42 | jamesh | it isn't something I need, and it is non-trivial |
| 14:04.20 | jamesh | the URL traversal alone for the admin interface is pretty heavily tied to the django ORM |
| 14:04.48 | johan | And I guess there's no proper interface for that as well |
| 14:57.42 | *** join/#storm andrea-bs (n=andrea@ubuntu/member/beeseek.developer.andrea-bs) |
| 15:45.32 | *** join/#storm niemeyer (n=niemeyer@200-181-177-142.ctame705.dsl.brasiltelecom.net.br) |
| 17:24.24 | *** join/#storm jamesh_ (n=james@canonical/launchpad/jamesh) |
| 17:42.31 | *** join/#storm dobee (n=dobee@85-124-200-100.static.xdsl-line.inode.at) |
| 18:12.18 | *** join/#storm jamesh__ (n=james@canonical/launchpad/jamesh) |
| 19:05.49 | *** join/#storm jamesh__ (n=james@canonical/launchpad/jamesh) [NETSPLIT VICTIM] |
| 19:05.49 | *** join/#storm jdobrien (n=jdobrien@238.205.119.70.cfl.res.rr.com) [NETSPLIT VICTIM] |
| 19:05.49 | *** join/#storm rockstar (n=rockstar@canonical/launchpad/rockstar) [NETSPLIT VICTIM] |
| 19:05.49 | *** join/#storm bac (n=bac@cpe-065-190-187-178.nc.res.rr.com) [NETSPLIT VICTIM] |
| 19:05.49 | *** join/#storm radix (n=r@wordeology.com) [NETSPLIT VICTIM] |
| 19:05.49 | *** join/#storm therve (n=th@88.191.37.10) [NETSPLIT VICTIM] |
| 19:19.13 | *** join/#storm oohlaf (n=olaf@unaffiliated/oohlaf) |
| 19:46.36 | *** join/#storm jamesh_ (n=james@canonical/launchpad/jamesh) |
| 20:57.35 | *** join/#storm dobee (n=dobee@85-124-200-100.static.xdsl-line.inode.at) |
| 21:34.05 | *** join/#storm mwhudson_ (n=mwh@118-93-51-162.dsl.dyn.ihug.co.nz) |
| 22:43.07 | *** join/#storm kov (n=kov@debian/developer/kov) |