| 00:00.15 | *** join/#storm wallflower (n=wallflow@ip-205-246-113-216.pool.grokthis.net) |
| 02:02.13 | *** join/#storm pyCube (n=jmayfiel@ip72-208-138-205.ph.ph.cox.net) |
| 03:26.46 | *** join/#storm jamesh (n=james@canonical/launchpad/jamesh) |
| 03:46.48 | *** join/#storm jkakar (n=jkakar@dhcp802-1-85.dsl.ucc-net.ca) |
| 03:50.11 | *** join/#storm pyCube (n=jmayfiel@ip72-208-138-205.ph.ph.cox.net) |
| 05:15.55 | pyCube | i have a class that has a thing = reference(), is it possible to do a find() for the type of thing that thing references? |
| 05:21.09 | jamesh | try describing it again |
| 05:21.47 | pyCube | ok.. |
| 05:22.22 | pyCube | class table(Storm): ref = Reference(id, anothertable.id) |
| 05:23.12 | pyCube | i want to query like, store.find(anothertable) |
| 05:23.33 | pyCube | but using self.ref |
| 05:28.19 | jamesh | if you have an instance of "table", can't you just get the corresponding "anothertable" instance with "foo.ref" directly? |
| 05:28.42 | pyCube | yes |
| 05:29.04 | pyCube | but i want to query all the other 'anothertable"s |
| 05:29.20 | jamesh | what do you mean by that exactly? |
| 05:29.32 | jamesh | all the "anothertable" rows matching what? |
| 05:30.32 | pyCube | at this point, all of them... just curious if i can query for a table class that i dont know, i only have an instance of it |
| 05:31.02 | jamesh | well |
| 05:31.07 | jamesh | type(instance) will give you the class |
| 05:31.19 | jamesh | or instance.__class__ if you prefer |
| 05:46.16 | pyCube | how does that help? |
| 06:11.23 | jamesh | pyCube: sorry, was doing something else. |
| 06:11.36 | jamesh | pyCube: I am still not sure what you want to do |
| 06:12.02 | jamesh | perhaps if you gave some details of your use case I could provide more help |
| 06:37.01 | jamesh | pyCube: is it possible that you want a ReferenceSet rather than a Reference? |
| 06:37.50 | pyCube | perhaps, if i can make a ref set like ReferenceSet(*, table.id) or similar |
| 06:38.08 | pyCube | actually, no |
| 06:38.09 | pyCube | hehe |
| 06:38.12 | jamesh | what would the "*" be? |
| 06:38.12 | pyCube | not that |
| 06:41.04 | pyCube | just a referenceset of all table, as if you did select * from table |
| 06:44.22 | jamesh | pyCube: okay. that's just store.find(classname), isn't it? |
| 06:44.36 | pyCube | yeah |
| 06:45.09 | jamesh | so do an @property that returns that |
| 06:45.13 | jamesh | if that's what you're after |
| 06:45.31 | jamesh | it doesn't sound like you're talking about a relation between the two tables |
| 06:46.58 | *** join/#storm bozzo (n=bozzo@BSN-77-49-15.dial-up.dsl.siol.net) |
| 06:48.59 | *** join/#storm bozzo (n=bozzo@BSN-77-49-15.dial-up.dsl.siol.net) |
| 06:53.19 | pyCube | is there a way to pass the class name to find as a string?, like store.find("table") ? |
| 06:53.42 | pyCube | like in Reference(), etc |
| 06:59.24 | *** join/#storm jukart (n=jukart@194.183.146.181) |
| 07:14.23 | jamesh | I don't think so |
| 07:14.26 | jamesh | why would you want to? |
| 07:16.23 | pyCube | for the same reason you can do it in Reference() |
| 07:50.04 | jamesh | pyCube: for Reference(), the reason for allowing strings is that the other class may not yet be defined |
| 07:50.45 | jamesh | if you are defining a property whose implementation calls store.find(), the code will be run after the classes have been instantiated |
| 07:51.38 | jamesh | so I don't think the same reasons hold |
| 07:54.43 | pyCube | what aboutr circular import issues? |
| 07:57.24 | jamesh | perform a local import |
| 08:02.57 | pyCube | right.. heh.. oh yeah |
| 08:33.06 | *** join/#storm mcella (n=michele@ip-171-30.sn3.eutelia.it) |
| 08:41.43 | *** join/#storm Key_Gena (i=lok@gateway/tor/x-263f54de325be392) |
| 09:47.05 | *** join/#storm pyCube (n=jmayfiel@ip72-208-138-205.ph.ph.cox.net) |
| 11:03.30 | *** join/#storm bozzo_ (n=bozzo@BSN-210-221-23.dial-up.dsl.siol.net) |
| 11:09.58 | *** join/#storm niemeyer (n=niemeyer@200-138-134-89.ctame705.dsl.brasiltelecom.net.br) |
| 11:32.42 | *** join/#storm gpolo (n=polo@189.7.21.145) |
| 14:07.40 | *** join/#storm bigdo1 (n=scmikes@72-197-8-8-arpa.cust.cinci.current.net) |
| 14:34.20 | radix | niemeyer: so, RETURNING |
| 14:34.39 | radix | niemeyer: is storm currently factored well to allow changing it to use RETURNING instead of a later SELECT? |
| 14:34.40 | niemeyer | RETURNING! |
| 14:34.49 | niemeyer | radix: It is |
| 14:34.53 | radix | awesome |
| 14:35.01 | niemeyer | radix: I *think* only the backends needs tweaking |
| 14:35.05 | niemeyer | backend |
| 14:35.57 | radix | excellent |
| 14:39.29 | niemeyer | radix: Even then, we should use some trick to prevent people with older servers to run into the same problem |
| 14:40.41 | niemeyer | The only thing I can think for now is a subselect |
| 14:40.57 | radix | oh, I see |
| 14:41.01 | radix | RETURNING id is only 8.1+? |
| 14:41.35 | niemeyer | 8.2+ |
| 14:41.39 | radix | oh |
| 14:42.28 | radix | so maybe we shouldn't even use RETURNING |
| 14:43.35 | niemeyer | radix: Why? |
| 14:43.50 | radix | if we can just use the subselect currval() |
| 14:43.59 | radix | well, I guess RETURNING can still save us an extra round trip |
| 14:44.06 | niemeyer | radix: Well, we're still saving a significant amount of ... right |
| 14:44.37 | niemeyer | radix: Besides being more correct |
| 14:44.55 | niemeyer | radix: It doesn't suffer from the same fragilities regarding sequences changing in triggers |
| 14:45.16 | radix | yeah |
| 14:47.15 | niemeyer | Okie |
| 14:47.24 | niemeyer | I have a test passing with a Returning expression.. |
| 14:47.31 | niemeyer | More after lunch |
| 15:06.10 | *** join/#storm mcella (n=michele@ip-171-30.sn3.eutelia.it) |
| 16:25.18 | *** join/#storm michelp (n=michelp@70.103.91.130) |
| 16:26.55 | *** join/#storm Key_Gena (i=lok@gateway/tor/x-f62e2659be762fe7) |
| 16:55.17 | *** join/#storm bigdog (n=scmikes@72-197-8-8-arpa.cust.cinci.current.net) |
| 17:25.21 | *** part/#storm gpolo (n=polo@189.7.21.145) |
| 17:27.42 | *** join/#storm dobee (n=dobeee@194.183.146.186) |
| 17:53.09 | Zenom | do you guys use a lot of zope stuff ? |
| 18:16.45 | jkakar | We use Zope 3 |
| 18:29.41 | Zenom | jkakar: how do you like it ? I have never personally used it myself |
| 18:30.25 | jkakar | Zenom: The learning curve is steep, but once you get past that it's a very nice system to work with. I'm enjoying it a lot. |
| 18:34.23 | Zenom | jkakar: using on high traffic sites? |
| 18:34.59 | jkakar | Zenom: Yes. launchpad.net uses Zope 3, for example. They get a fair amount of traffic. :) |
| 18:35.08 | Zenom | interesting |
| 18:38.04 | Zenom | just seems more time consuimg |
| 18:38.10 | Zenom | with all the config files I have seen so far etc |
| 18:42.07 | jkakar | Yeah. People have mixed feelings about ZCML. Basically, the idea is that you separate configuration of a component from the implementation of a component; this, when done well, makes components easily reusable. |
| 18:42.21 | jkakar | I generally find that separation of concerns has benefit. Sometimes I find ZCML a bit boilerplate-y, but overall a good thing. |
| 19:06.58 | *** join/#storm dobee (n=dobeee@81-223-53-162.dornbirn.xdsl-line.inode.at) |
| 19:07.26 | Zenom | is there a way to create the database based on the schema? for example with sqlite? |
| 19:08.14 | radix | Zenom: you mean based on the storm classes? no |
| 19:08.29 | Zenom | right, ok so everything has to be made by hand |
| 19:08.52 | weatherman_ | storm/insert-returning r191 committed by gustavo@niemeyer.net |
| 19:08.52 | weatherman_ | Modified the postgres database backend to use the RETURNING extension |
| 19:08.52 | weatherman_ | of the INSERT statement to obtain the primary key values which were |
| 19:08.56 | weatherman_ | inserted, and then assign them with the respective variables. |
| 19:09.04 | weatherman_ | This extension is only supported in Postgres >= 8.2. Old versions |
| 19:09.08 | weatherman_ | will continue to work fine with the previously existent mechanism, |
| 19:09.12 | weatherman_ | which requires one additional query to be performed. |
| 19:09.21 | radix | woot |
| 19:19.50 | *** join/#storm bigdog (n=scmikes@72-197-8-8-arpa.cust.cinci.current.net) |
| 19:27.40 | *** join/#storm bigdo1 (n=scmikes@72-197-8-8-arpa.cust.cinci.current.net) |
| 20:52.54 | Zenom | anyone currently working on a way to take established storm model classes and forming them into generated html forms? :D |
| 20:55.34 | radix | I don't think so |
| 20:56.36 | Zenom | does storm have ways to define unique fields, not-null fields? |
| 20:57.17 | Zenom | and I wonder how hard it would be to create a form generator for storm |
| 20:57.27 | radix | Zenom: well, allow_none=False is a way to say that None can't be a value for a property |
| 20:57.45 | radix | it doesn't have a way to enforce uniqueness though, it relies on the database for that |
| 20:57.52 | Zenom | gotcha |
| 20:58.30 | Zenom | would be neat to have some sort of form generator based on storm classes though, will have to research, i wonder if I could pull it off, lol |
| 20:59.34 | Zenom | but then again if there is no validation in the model, the validation would have to be in a separate class like formencode |
| 21:15.09 | *** join/#storm Fujitsu (n=fujitsu@ubuntu/member/fujitsu) |
| 21:26.22 | aa_ | Zenom: if its any use to you I do form generation in pygtk + storm |
| 21:26.37 | aa_ | execpt one thing, my form generation schemas are totally independent of the model |
| 21:27.15 | aa_ | this is like my 4th iteration, and one thing I have set in stone is that the form generating schemas are totally separate from the model definitions |
| 21:27.32 | aa_ | and then I even have a separate layer for validation |
| 21:27.40 | aa_ | (why am I telling you this?) |
| 21:28.14 | aa_ | because I honestly think that form generation straight from model definitions (by adding a load of clunk) leaves you in a horrible non-portable mess |
| 21:28.39 | aa_ | non-portable in the case that when/if you change ORM, or the ORM API changes, you are pretty screwed |
| 21:28.55 | aa_ | and it's not maintainable either |
| 21:29.09 | aa_ | just my 2c :) |
| 21:29.43 | *** join/#storm dobee (n=dobeee@81-223-53-162.dornbirn.xdsl-line.inode.at) |
| 21:47.51 | weatherman_ | storm/insert-returning r192 committed by gustavo@niemeyer.net |
| 21:47.51 | weatherman_ | Rather than retrieving all values from the database after an object |
| 21:47.52 | weatherman_ | is flushed, retrieve only the primary key, if necessary. All other |
| 21:47.56 | weatherman_ | values are set to AutoReload, and will only be retrieved once the |
| 21:48.00 | weatherman_ | one of them is touched. |
| 21:48.08 | weatherman_ | This should improve the performance of Storm-based applications |
| 21:48.12 | weatherman_ | significantly, as inserted values are only really retrieved if |
| 21:48.16 | weatherman_ | needed. In cases where the primary key is preset explicitly or |
| 21:48.20 | weatherman_ | by the backend during the insertion, this will prevent the second |
| 21:48.24 | weatherman_ | query entirely. In other cases, it will mean a fast index lookup. |
| 21:59.42 | *** join/#storm dobee (n=dobeee@81-223-53-162.dornbirn.xdsl-line.inode.at) [NETSPLIT VICTIM] |
| 21:59.42 | *** join/#storm bigdo1 (n=scmikes@72-197-8-8-arpa.cust.cinci.current.net) [NETSPLIT VICTIM] |
| 21:59.42 | *** join/#storm weatherman_ (n=weatherm@dhcp802-1-85.dsl.ucc-net.ca) [NETSPLIT VICTIM] |
| 21:59.42 | *** join/#storm WebMaven (n=webmaven@nv-65-173-64-133.dhcp.embarqhsd.net) [NETSPLIT VICTIM] |
| 21:59.51 | *** join/#storm radix (n=radix@70.91.133.157) [NETSPLIT VICTIM] |
| 22:00.05 | *** join/#storm michelp (n=michelp@70.103.91.130) [NETSPLIT VICTIM] |
| 22:00.05 | *** join/#storm pyCube (n=jmayfiel@ip72-208-138-205.ph.ph.cox.net) [NETSPLIT VICTIM] |
| 22:00.05 | *** join/#storm ajmitch (n=ajmitch@ubuntu/member/ajmitch) [NETSPLIT VICTIM] |
| 22:17.31 | weatherman_ | storm/insert-returning r193 committed by gustavo@niemeyer.net |
| 22:17.31 | weatherman_ | Changed get_insert_identity() in the PostgreSQL to prevent a |
| 22:17.32 | weatherman_ | pathological case where performing table.id = currval(...) will |
| 22:17.36 | weatherman_ | ignore the index for table.id. This is only meaninful for |
| 22:17.40 | weatherman_ | versions of PostgreSQL < 8.2, as otherwise the INSERT+RETURNING |
| 22:17.44 | weatherman_ | mechanism is used. |
| 22:17.58 | Zenom | nice to see storm progressing :) |
| 22:18.36 | niemeyer | That's what real world usage makes for you :-) |
| 22:18.50 | Zenom | aa_: thanks for the info, yeah i see what you mean about the validation from the model etc. |
| 22:18.59 | Zenom | niemeyer: yeah, thank god looks like you guys use postgresql |
| 22:19.00 | Zenom | lol |
| 22:19.21 | niemeyer | Zenom: Indeed.. |
| 22:19.26 | niemeyer | Zenom: That's a pretty controversial case.. |
| 22:19.47 | niemeyer | Zenom: We would probably not even face that kind of issue if we were using MySQL :-) |
| 22:20.15 | Zenom | yeah there is a good and bad for both |
| 22:20.28 | Zenom | i like postgresql but i have found it really hard to find orm's that work the right way with it |
| 22:20.56 | Zenom | so far I think storm is the most straight forward and flexible |
| 22:21.05 | Zenom | although I wish order bys and sorting were a little more straight forward |
| 22:21.30 | Zenom | ie., with things like find(News).order_by('title DESC") |
| 22:21.31 | niemeyer | Zenom: Hmm |
| 22:21.32 | Zenom | or something |
| 22:21.38 | niemeyer | Zenom: How can it be more straightforward? |
| 22:21.45 | niemeyer | Zenom: I mean, you say order_by(column) |
| 22:21.52 | niemeyer | Zenom: Is there something simpler than that? :) |
| 22:21.54 | dobee | niemeyer: mysql has the same problem |
| 22:21.55 | Zenom | well but thats on the result |
| 22:22.00 | niemeyer | dobee: Oh, really? |
| 22:22.04 | Zenom | i was thinking before the result |
| 22:22.16 | niemeyer | Zenom: You can also specify default ordering |
| 22:22.16 | Zenom | or am i missing something |
| 22:22.16 | dobee | yes, we found out (but not with storm) |
| 22:22.24 | niemeyer | dobee: Do you have any details? |
| 22:22.34 | dobee | select max(ident) is faster than the current value |
| 22:22.46 | niemeyer | dobee: I'd like to see if Storm is affected by it, and try to do something about it as well |
| 22:22.47 | dobee | niemeyer: sec |
| 22:23.10 | Zenom | niemeyer: can you set a default limit too? |
| 22:23.51 | niemeyer | Zenom: Nope.. that seems a bit unusual |
| 22:24.01 | Zenom | why? If I am looking up transactions |
| 22:24.09 | Zenom | why would I want to return 1 million transactions so to speak |
| 22:24.15 | Zenom | when I am showing say 10 per page |
| 22:24.41 | radix | Zenom: you realize you can say .order_by(Desc(News.title)), right? |
| 22:24.57 | Zenom | radix: is that on the find()? |
| 22:25.00 | radix | yeah |
| 22:25.03 | niemeyer | Zenom: Because: 1) You don't want to show every object in your system in pages, 2) You don't have to return all 1 million objects when you iterate over them, etc |
| 22:25.05 | Zenom | oh, no didn't know that |
| 22:25.13 | Zenom | i was thinking i had to do lik |
| 22:25.24 | Zenom | news = find(News) |
| 22:25.26 | Zenom | then sort it |
| 22:25.40 | Zenom | according to the examples on the wiki thats how I understood it i guess |
| 22:26.01 | radix | can you point at specific examples? |
| 22:26.03 | Zenom | niemeyer: but isn't that taking up more memory? By storing all 1 million rows? |
| 22:26.26 | Zenom | https://storm.canonical.com/Tutorial#head-d1ed8578d511f42e79b2ff3e9b67051a1d58817f |
| 22:26.35 | Zenom | ordering and limiting results |
| 22:26.48 | Zenom | that seems confusing and doesn't necessarily state you can append .order_by to the find i guess |
| 22:26.54 | Zenom | maybe its just my lack of undertsanding of it all too |
| 22:27.03 | radix | that's just basic Python stuff :) |
| 22:27.09 | radix | x = foo() |
| 22:27.13 | radix | y = x.blah() |
| 22:27.14 | Zenom | well but you have result.order_by() |
| 22:27.16 | radix | is the same as y = foo().blah() |
| 22:27.22 | Zenom | so you are doing it to the list i guess the list is the list is the list |
| 22:27.31 | radix | the result of find() is not a list |
| 22:27.31 | Zenom | wether i am appending to the find() or doing it to the result |
| 22:27.44 | Zenom | well object |
| 22:27.48 | Zenom | or whatever it is heh |
| 22:28.09 | Zenom | but now that I know i can append to the find |
| 22:28.12 | Zenom | thats fine with me :) |
| 22:28.18 | radix | hooray |
| 22:28.30 | Zenom | i was thinking to sort I had to do like the whole example shown there :) |
| 22:28.43 | Zenom | [employee.name for employee in result.order_by(Employee.name)[:2]] |
| 22:28.53 | Zenom | can you limit in the same way radix? |
| 22:29.01 | Zenom | or do you have to limit it from the result as shown? |
| 22:29.02 | radix | yes |
| 22:29.14 | radix | store.find(Employee)[:2] works |
| 22:29.22 | Zenom | very cool , ok |
| 22:29.36 | Zenom | sorry i guess I didn't realize that worked haha |
| 22:31.06 | Zenom | i thought those parts were going to be more complicated lol |
| 22:31.29 | dobee | niemeyer: dunno if it applies to storm but "select last_insert_id() from ..." is extremely slow |
| 22:31.38 | Zenom | if there is anything i can help with documentation wise in the project or anything let me know |
| 22:31.40 | Zenom | i will try to help out |
| 22:32.35 | niemeyer | dobee: Let me check |
| 22:33.05 | niemeyer | dobee: Probably not.. we get it from the cursor |
| 22:33.18 | dobee | ah ok |
| 22:33.48 | niemeyer | dobee: Hmm.. |
| 22:34.03 | niemeyer | dobee: Would you like to review a changeset we're going to integrate? |
| 22:34.14 | niemeyer | dobee: (and would you have the time to do it now) |
| 22:34.25 | dobee | its midnight here |
| 22:34.31 | niemeyer | dobee: Ouch :) |
| 22:34.38 | niemeyer | dobee: Ok.. thanks even then |
| 22:35.02 | dobee | i can test the stuff with our test cases tomorrow |
| 22:35.05 | dobee | if that helps |
| 22:36.27 | niemeyer | dobee: That'd be cool, definitely |
| 22:36.48 | niemeyer | dobee: We've run the Landscape test suite on it as well, but in that case, the more the better |
| 22:36.53 | niemeyer | dobee: Thanks in advance |
| 22:37.04 | dobee | do you have a branch url |
| 22:37.19 | niemeyer | dobee: Will probably be on trunk by tomorrow |
| 22:37.23 | dobee | k |
| 22:37.56 | Zenom | we are thinking about using landscape when we reload all our servers |
| 22:38.03 | Zenom | or as, we reload them i should say |
| 22:38.34 | niemeyer | Zenom: Cool! |
| 22:38.46 | Zenom | like 35 servers and growing heh |
| 22:39.20 | niemeyer | Zenom: They'll be very welcome :) |
| 22:40.01 | Zenom | when is the next lts coming out? |
| 22:40.32 | niemeyer | Zenom: 6 months |
| 22:40.48 | Zenom | cool |
| 22:42.15 | radix | [1] execute.__doc__ = Connection.__doc__ |
| 22:42.16 | radix | I think you probably meant Connection.execute.__doc__. |
| 22:42.16 | radix | However, I'd much appreciate if the docstring were something like |
| 22:42.16 | radix | """ |
| 22:42.16 | radix | Similar to L{Connection.execute}, but |
| 22:42.17 | radix | <reasons that we need a specialized implementation>. |
| 22:42.20 | radix | """ |
| 22:44.14 | niemeyer | radix: It was originally like that, but I've wondered if that'd be useful for people doing e.g. help(connection.execute) |
| 22:44.14 | radix | (^^niemeyer) |
| 22:44.39 | niemeyer | radix: So I copied over and used a comment for the explanation |
| 22:45.05 | niemeyer | radix: Would you prefer to change it even then? |
| 22:46.01 | radix | niemeyer: I would still do it the way I described, as long as there's an explicit reference to the common thing they should read, but this is of course not an important issue |
| 22:46.14 | niemeyer | radix: Ok, I'll change it then |
| 22:46.57 | radix | man I wish I could use bzr-gtk :( |
| 22:48.10 | radix | [2] Postgres._version = tuple(int(x) for x in server_version.split(".")) |
| 22:48.20 | niemeyer | radix: |
| 22:48.23 | niemeyer | <PROTECTED> |
| 22:48.23 | niemeyer | <PROTECTED> |
| 22:48.23 | niemeyer | <PROTECTED> |
| 22:48.23 | niemeyer | <PROTECTED> |
| 22:48.23 | niemeyer | <PROTECTED> |
| 22:48.25 | niemeyer | radix: ? |
| 22:48.45 | radix | What if they've got multiple stores/Databases pointing at different Postgres servers with different versions? |
| 22:48.58 | radix | niemeyer: re [1]: looks good |
| 22:49.17 | niemeyer | radix: Oh, ouch.. |
| 22:49.34 | niemeyer | radix: I originally had it in the connection, then I moved it to the database, which is right, but definitely not in the calss |
| 22:49.34 | niemeyer | class |
| 22:49.38 | niemeyer | radix: Thanks for catching it |
| 22:49.50 | radix | cool |
| 22:50.36 | niemeyer | radix: Does s/Posgres/self/ sound correct? |
| 22:51.37 | radix | yes |
| 22:51.50 | radix | after having thought about it for about 60 seconds :) |
| 22:53.25 | niemeyer | Ok, tests like it |
| 22:54.23 | jkakar | niemeyer: test_wb_currval_based_identity doesn't seem to actually verify that the right query is run for pre-8.2 versions of PostgreSQL. Seems like it should. |
| 22:55.08 | niemeyer | jkakar: Hmm.. I don't get this one |
| 22:55.17 | niemeyer | jkakar: Hmmm |
| 22:55.19 | niemeyer | jkakar: Oh, ok |
| 22:55.21 | niemeyer | jkakar: So |
| 22:55.22 | jkakar | niemeyer: So, maybe I'm misinterpreting what the test is about? |
| 22:55.33 | niemeyer | jkakar: Let me see it |
| 22:57.18 | niemeyer | jkakar: Oh.. so we have two different sets of tests for the same subject |
| 22:57.42 | niemeyer | jkakar: This one test verifies that the statement which is executed for the currval-based system actually works |
| 22:58.06 | niemeyer | jkakar: IOW, it's somewhat of an integration test between the store, the database, and the backend |
| 22:58.34 | niemeyer | jkakar: We have specific tests in tests/databases/postgres.py which verifies when RETURNING is or is not used |
| 22:59.00 | jkakar | niemeyer: Right, I get that part. But, unless I'm missing something, there's nothing that asserts, in that test, that it's using currval, instead of RETURNING. I'm still making my way through the changes, so maybe my comment is premature. |
| 22:59.21 | niemeyer | jkakar: Specifically, test_wb_execute_insert_returning_with_old_postgres |
| 22:59.34 | niemeyer | Which is probably misnamed |
| 22:59.46 | niemeyer | It's now named |
| 22:59.48 | niemeyer | test_wb_execute_insert_returning_not_used_with_old_postgres |
| 23:00.08 | niemeyer | jkakar: That: self.assertFalse(variable1.is_defined()) |
| 23:01.30 | niemeyer | jkakar: Hmm.. I think I can also add an additional assertion on the result of INSERT |
| 23:01.32 | niemeyer | Which should be 1 |
| 23:01.35 | niemeyer | Let me try that |
| 23:02.40 | niemeyer | Hmm.. doesn't work as I expected |
| 23:09.51 | radix | [3] + def test_execute_insert_returning_without_columns(self): |
| 23:09.51 | radix | + """Without primary_variables, INSERT+RETURNING won't work.""" |
| 23:09.57 | radix | I think you mean "Without primary_columns" |
| 23:09.58 | radix | ? |
| 23:11.29 | niemeyer | Oops |
| 23:11.48 | niemeyer | They're both inverted |
| 23:11.50 | radix | actually that description is pretty confusing |
| 23:12.21 | radix | I mean, I don't really see what that test has to do with "RETURNING" at all |
| 23:12.41 | niemeyer | It's not using it.. |
| 23:12.43 | niemeyer | <PROTECTED> |
| 23:12.49 | radix | oops sorry |
| 23:12.55 | niemeyer | Considering that you and jkakar didn't get this, I think it's obscure :( |
| 23:13.01 | radix | no actually I get it |
| 23:13.09 | radix | I was just being stupid |
| 23:13.15 | radix | I *did* grok the assertFalse |
| 23:13.25 | radix | I just forgot momentarily that execute() is where the magic happens |
| 23:13.57 | jkakar | niemeyer: I'm still partway through the code; I probably just spoke up prematurely. |
| 23:14.29 | jkakar | niemeyer: I'm not very happy with this test_wb_version, but I can't think of a better way to do it. I would prefer to assert against a hard-coded value. The assert here is basically the same code in the implementation. |
| 23:14.35 | niemeyer | I guess I can easily put mocker to work here, if necessary |
| 23:14.42 | niemeyer | By expecting a call on get_insert_identity |
| 23:14.48 | jkakar | ie: ".".join(str(x) for x in version) |
| 23:15.25 | niemeyer | jkakar: Well, that's what it's asserting :) |
| 23:15.31 | jkakar | niemeyer: Yeah, I know. :) |
| 23:15.53 | radix | btw map(str, version) is shorter ;P |
| 23:15.57 | jkakar | niemeyer: I just tend to be wary when a test basically just copies an expression from the implementation into the test. But, like I said, I can't think of a better way right now. |
| 23:16.06 | niemeyer | radix: Wow.. brilliant :-) |
| 23:17.15 | niemeyer | jkakar: Well, it didn't actually copy the expression |
| 23:17.24 | niemeyer | jkakar: They're precisely the opposite of each other |
| 23:17.40 | niemeyer | jkakar: and we test the types of the value |
| 23:17.46 | jkakar | niemeyer: Hehe, yeah, I guess you're right. |
| 23:17.49 | niemeyer | jkakar: So there's something else going on there |
| 23:18.17 | jkakar | niemeyer: I guess I really want to see assertEquals(server_version, "8.2.1"), but that of course won't work. |
| 23:18.56 | niemeyer | jkakar: Right |
| 23:21.01 | niemeyer | jkakar: Hmm.. actually, in a certain way that's what we're doing |
| 23:21.12 | niemeyer | jkakar: Since we get server_version from postgres untouched |
| 23:21.29 | Zenom | you guys know of an sqlite gui for mac? |
| 23:21.34 | jkakar | niemeyer: Hmm. Yeah, I think you're right. |
| 23:21.37 | radix | ok yeah, this second revision is hard |
| 23:21.40 | Zenom | don't want to install postgresql locally if I don't have to |
| 23:21.58 | radix | Zenom: why do you want a gui? |
| 23:22.01 | niemeyer | radix: Sorry :( |
| 23:22.13 | Zenom | radix just makes it easier |
| 23:22.15 | Zenom | being lazy |
| 23:22.16 | Zenom | lol |
| 23:22.32 | niemeyer | radix: Let me explain the idea.. maybe it helps |
| 23:22.53 | niemeyer | Before, _fill_missing_values() was a big and ugly function |
| 23:22.55 | radix | Zenom: what does it make easier? I mean, what do you want to use it for? |
| 23:23.14 | radix | Zenom: if you just want to run the storm tests, if you have python2.5 it will work because os x comes with sqlite and python 2.5 comes with pysqlite |
| 23:23.41 | niemeyer | It did several things: validated cached objects, and resolved lazy values to their database real values |
| 23:23.52 | niemeyer | Now, these features were split |
| 23:24.31 | Zenom | radix: i just didn't want to generate the tables manually |
| 23:24.34 | Zenom | just being lazy is all |
| 23:24.37 | niemeyer | _fill_missing_values() should only retrieve the primary key values, if they're missing, and set everything else that is missing to AutoReload |
| 23:24.47 | Zenom | ie., didn't want to do CREATE TABLE news ( ... ) lol |
| 23:24.54 | radix | Zenom: oh, ok :) |
| 23:25.50 | niemeyer | Then, _resolve_lazy_value actually resolves values |
| 23:25.59 | niemeyer | and _validate_cache does the cache validation step |
| 23:26.13 | niemeyer | Notice that _fill_missing_values() now is only called in _flush_one() |
| 23:26.18 | jkakar | Storm needs more docstrings. :) |
| 23:26.18 | radix | right, cool |
| 23:26.27 | jkakar | But that explanation does really help, thanks. |
| 23:26.27 | radix | actually I'm pretty happy with the docstring for _fill_missing_values |
| 23:27.19 | jkakar | I would like to write meaningful docstrings for the tests, actually. I often find I'm reading them and feel that I'm missing an important detail that isn't obvious from the implementation. Eventually, the light turns on and I usually remember/figure it out. :) |
| 23:27.38 | radix | niemeyer: I suggest renaming it to _fill_missing_primary_values, or at least updating the first line of the docstring to say as such |
| 23:27.50 | radix | (now that it's changed) |
| 23:28.15 | niemeyer | radix: It actually "fills" other variables as well |
| 23:28.20 | radix | although maybe not, since it does... |
| 23:28.21 | radix | right |
| 23:28.35 | radix | never mind, the docstring is already perfect |
| 23:28.50 | radix | interactive reviews are bad, they give me a chance to blabber without accumulating all of my thoughts :) |
| 23:30.00 | jkakar | Hehe, I feel the same way. |
| 23:30.15 | niemeyer | <PROTECTED> |
| 23:30.16 | niemeyer | <PROTECTED> |
| 23:30.16 | niemeyer | <PROTECTED> |
| 23:30.16 | niemeyer | <PROTECTED> |
| 23:30.16 | niemeyer | <PROTECTED> |
| 23:30.16 | niemeyer | <PROTECTED> |
| 23:30.18 | niemeyer | <PROTECTED> |
| 23:34.45 | jkakar | Nice |
| 23:36.36 | Zenom | sqlite only has 4 column types? text, integer, blob and real? |
| 23:37.08 | niemeyer | Zenom: Hmm.. something close to that :) |
| 23:37.17 | radix | man this second revision is actually really nice |
| 23:37.17 | Zenom | rofl |
| 23:37.23 | niemeyer | Zenom: It actually has a different idea about column types |
| 23:37.28 | radix | it actually only has two column types |
| 23:37.36 | niemeyer | Zenom: The documentation will help in that case |
| 23:37.54 | Zenom | yeah i am looking at it now, lol |
| 23:38.23 | niemeyer | There's that "affinity" thing |
| 23:42.14 | jkakar | niemeyer: Okay, I've read over this code carefully. I suspect there are things I'm missing, but I think I understand most of the changes. I think I'm happy to say +1 on it. |
| 23:42.41 | niemeyer | jkakar: Thanks for checking it out! |
| 23:43.17 | jkakar | niemeyer: No problem! It looks pretty cool. I was pleased to see that making a, seemingly significant, change like adding the RETURNING stuff was quite straight-forward. |
| 23:44.01 | radix | man this second revision is *awesome* :) |
| 23:45.09 | weatherman_ | storm/insert-returning r194 committed by gustavo@niemeyer.net |
| 23:45.09 | weatherman_ | Applying reviews from Jamu and Chris. |
| 23:45.28 | radix | and the third is short and sweet |
| 23:45.32 | radix | niemeyer: +1! |
| 23:46.03 | niemeyer | WOOT! |
| 23:46.59 | jkakar | Oh, this is so exciting. |
| 23:55.29 | weatherman_ | storm/trunk r191 committed by gustavo@niemeyer.net |
| 23:55.30 | weatherman_ | Merged insert-returning branch [r=radix,jkakar] |
| 23:55.34 | weatherman_ | This branch implements three important changes: |
| 23:55.42 | weatherman_ | - It changes the flushing mechanism to not load values inserted |
| 23:55.46 | weatherman_ | <PROTECTED> |
| 23:55.50 | weatherman_ | <PROTECTED> |
| 23:55.58 | weatherman_ | - It implements support in the postgres backend to use the RETURNING |
| 23:56.02 | weatherman_ | <PROTECTED> |
| 23:56.06 | weatherman_ | <PROTECTED> |
| 23:56.14 | weatherman_ | - It prevents a pathological case which happens when a statement like |
| 23:56.18 | weatherman_ | <PROTECTED> |
| 23:56.22 | weatherman_ | <PROTECTED> |
| 23:56.26 | weatherman_ | <PROTECTED> |