| *** leseb has quit IRC | 01:33 | |
| *** stevemar has quit IRC | 01:36 | |
| *** morganfainberg is now known as morganfainberg_Z | 01:42 | |
| *** topol has joined #openstack-keystone | 01:42 | |
| *** morganfainberg_Z is now known as morganfainberg | 01:52 | |
| *** david-lyle has quit IRC | 02:00 | |
| *** mberlin has quit IRC | 02:03 | |
| *** mberlin has joined #openstack-keystone | 02:20 | |
| *** dstanek has quit IRC | 02:32 | |
| *** topol has quit IRC | 02:35 | |
| *** thedodd has joined #openstack-keystone | 02:37 | |
| ayoung-zzz | morganfainberg, are you actually there? | 02:45 |
|---|---|---|
| morganfainberg | ayoung-zzz, sorta | 02:45 |
| ayoung-zzz | morganfainberg, what does this line of code do? | 02:47 |
| ayoung-zzz | link in a sec | 02:47 |
| ayoung-zzz | https://github.com/openstack/keystone/blob/master/keystone/contrib/revoke/model.py#L193 | 02:47 |
| ayoung-zzz | note that subnode is the collection in the for statement above | 02:47 |
| ayoung-zzz | I thought I understood this code, but YorikSar did something beyond my Ken | 02:48 |
| ayoung-zzz | morganfainberg, what does python do in an iteration if you change the variable that is used in a for statement? I know that if you modify the underlying collection, you get an error | 02:48 |
| morganfainberg | ok so the only place where you need to worry about oddities is when you're dealing with the actual iterator changing or assigning a value inside a separate scope | 02:49 |
| morganfainberg | for loop isn't _actuall_ a separate scope | 02:49 |
| morganfainberg | so what you're doing is you're rebinding subnode to the current bundle, so that when you hit the else you have something to iterate over | 02:50 |
| morganfainberg | that is assuming the bundle has changed. | 02:50 |
| * morganfainberg personally really doesn't like for/else | 02:50 | |
| ayoung-zzz | morganfainberg, so it is "first match" semantics here? If subnode gets reassigned, restart the looping? And the origianal loop never gets finished> | 02:50 |
| ayoung-zzz | ? | 02:50 |
| morganfainberg | in this case, you could remove the else and back-dent the second for loop and it would probably be easier to read | 02:51 |
| morganfainberg | oooh wait a sec | 02:51 |
| morganfainberg | i was reading the outer loop | 02:51 |
| morganfainberg | hold on here | 02:51 |
| ayoung-zzz | I liked it better when it was recursive, but I think I got caught up in the elegance of this rewrite | 02:51 |
| morganfainberg | oh | 02:51 |
| morganfainberg | ok, i _think_ the original for isn't impacted by the later rebind | 02:52 |
| morganfainberg | because the iterator has already been created | 02:52 |
| ayoung-zzz | the origianl for is on NAMEs | 02:52 |
| morganfainberg | line 181 | 02:52 |
| ayoung-zzz | ok | 02:52 |
| morganfainberg | if you were to change the contents of "subnode" that would be a problem | 02:52 |
| morganfainberg | in this case we're changing the reference of what subnode is. | 02:53 |
| ayoung-zzz | this is icky | 02:53 |
| ayoung-zzz | requires way to much deep python know-how to follow the logic | 02:53 |
| ayoung-zzz | like the filter statment on 190 | 02:53 |
| morganfainberg | oh wow. yeah you're changing the loop mid-run | 02:54 |
| morganfainberg | oh man. | 02:54 |
| morganfainberg | but you're not changng the references | 02:54 |
| morganfainberg | ok here this explains it better | 02:54 |
| ayoung-zzz | the problem is that the code seems to be failing once I move it to the client. I suspect it has something to do with marshalling the events themselves and building the tree with the reconstitued events | 02:55 |
| ayoung-zzz | but this is failing http://fpaste.org/85781/94938564/ | 02:56 |
| ayoung-zzz | and I can't understand the code | 02:57 |
| *** zhiyan_ is now known as zhiyan | 02:57 | |
| ayoung-zzz | morganfainberg, anyway, waiting on your explanation, but I think we need to make this code more readable. | 02:57 |
| morganfainberg | ayoung-zzz, http://paste.openstack.org/show/73578/ | 02:58 |
| morganfainberg | ayoung-zzz, so the iteration never changes (the loop on 181) but each time you run through the iteration, you're potentially changing the value of subnode for when you hit else | 02:58 |
| ayoung-zzz | ah | 02:58 |
| morganfainberg | the else is hit if there isn't an explicit break in the for loop (in this case the return isn't called) | 02:58 |
| ayoung-zzz | potentially....which is way too subtle | 02:58 |
| morganfainberg | this becomes a lot easier to read if you remove the else and back-dent that subnode for loop | 02:59 |
| *** zhiyan is now known as zhiyan_ | 02:59 | |
| morganfainberg | ayoung-zzz, sure the changing of the value is a bit odd, but i think what makes it hard to grasp is both that plus for/else | 02:59 |
| morganfainberg | for / else should (imo) always be a call to another method, and never done | 03:00 |
| morganfainberg | i just don't like reading it, nor will most python developers feel it makes it easier to read | 03:00 |
| *** mberlin1 has joined #openstack-keystone | 03:01 | |
| ayoung-zzz | that code is supposed to be leaf node logic | 03:01 |
| morganfainberg | well it is | 03:01 |
| ayoung-zzz | test still fails...but no surprise there | 03:01 |
| morganfainberg | it's just hard to grok | 03:02 |
| morganfainberg | and i agree it should be more readable | 03:02 |
| ayoung-zzz | I copied the code from server to client....I don;t like that it fails, and I really don't like how hard it is to follow | 03:02 |
| ayoung-zzz | I think I want to clean up the server code before dragging it on over. | 03:02 |
| morganfainberg | heck, just using a variable not called "subnode" that bundle is assigned to would make it easier to read | 03:03 |
| ayoung-zzz | OK.. that can happend later, not going to hjappend to night | 03:03 |
| *** mberlin has quit IRC | 03:03 | |
| ayoung-zzz | bundle is also a bad name. He origianlly used "faggot" as, I am guessing, he is not a native english speaker | 03:03 |
| morganfainberg | oh dear | 03:03 |
| ayoung-zzz | technically, it was a correct use of the word, but in its origianl meaning which is long since forgotten | 03:04 |
| morganfainberg | yeah | 03:04 |
| ayoung-zzz | kinda like fascii | 03:04 |
| ayoung-zzz | let me see what happens if I change the logic on the server to look like what you suggest... | 03:04 |
| morganfainberg | 2 changes i recommend, 1: remove the for/else and backdent | 03:05 |
| morganfainberg | 2: don't reuse "subnode" in the "leaf" loop | 03:05 |
| ayoung-zzz | tests still pass with it | 03:06 |
| morganfainberg | yeah | 03:07 |
| morganfainberg | i think recursive might have been easier to read :P | 03:10 |
| ayoung-zzz | yeah. I might move back to that | 03:10 |
| morganfainberg | so this imo is way easier to read: http://paste.openstack.org/show/73581/ still not 100% clear, but a comment will cover that | 03:11 |
| morganfainberg | and this is probably the best (still needs comments) without going back to recursion | 03:13 |
| morganfainberg | http://paste.openstack.org/show/73582/ | 03:13 |
| ayoung-zzz | morganfainberg, that subnode=bundle must be more significant | 03:13 |
| morganfainberg | oh ohhhh | 03:13 |
| morganfainberg | actually it is | 03:13 |
| morganfainberg | outter loop | 03:13 |
| morganfainberg | *derp* | 03:13 |
| morganfainberg | it affects subsequent runs of the outerloop | 03:13 |
| ayoung-zzz | right | 03:14 |
| morganfainberg | actually... i wonder if this could possibly be an issue. | 03:14 |
| ayoung-zzz | almoste certainly | 03:14 |
| morganfainberg | it seems to me we are eliminating a ton of possible places to look in the revoke_map | 03:14 |
| morganfainberg | potentially not checking revocations correctly | 03:14 |
| ayoung-zzz | so whatever subnode is set to in the last iteration through the loop is what will be iterated over the next go round | 03:15 |
| morganfainberg | yep | 03:15 |
| ayoung-zzz | no, I don;t think we miss anything | 03:15 |
| ayoung-zzz | that is why it was added to the bundle | 03:15 |
| morganfainberg | oh i see | 03:16 |
| ayoung-zzz | bundle gets filtered, but not reset | 03:16 |
| morganfainberg | keeps adding to it. | 03:16 |
| morganfainberg | so you end up with the data from all the _NAMES by the end | 03:16 |
| ayoung-zzz | yeah, and the else block only gets executed once there is nothing left in bundle | 03:16 |
| ayoung-zzz | the filter removes any internal nodes that don't have leaf nodes | 03:17 |
| morganfainberg | still can remove the else and backdent w/o issue | 03:17 |
| morganfainberg | can't stop the re-use of subnode though | 03:17 |
| ayoung-zzz | It changes the logic | 03:18 |
| morganfainberg | no it doesn't | 03:18 |
| ayoung-zzz | if you remove, that code gets executed more | 03:18 |
| morganfainberg | backdenting wouldn't change anything | 03:18 |
| morganfainberg | nope | 03:18 |
| ayoung-zzz | I think the else is a performance tune | 03:18 |
| ayoung-zzz | yeah, in his version, it only gets executed if subnode is empty | 03:18 |
| ayoung-zzz | in yours it gets executed each time trhough the NAMES loop | 03:19 |
| ayoung-zzz | it just never matches | 03:19 |
| morganfainberg | maybe? | 03:19 |
| morganfainberg | http://paste.openstack.org/show/73583/ | 03:19 |
| morganfainberg | no it gets executed in either case | 03:19 |
| ayoung-zzz | its only an empty eva of the for loop that gets executed, so no big hit to remove | 03:19 |
| morganfainberg | unless bundle is legitimately empty after the filter, which case the return | 03:19 |
| morganfainberg | the else is run after the for loop continues, but if you had a break, the else wouldn't run | 03:20 |
| morganfainberg | in this case the break is a return, so the whole function is exited, | 03:20 |
| morganfainberg | the backdent wouldn't matter | 03:20 |
| ayoung-zzz | wow, I misunderstood that | 03:20 |
| morganfainberg | s/continues/completes | 03:20 |
| morganfainberg | for/else is not straightforward at all | 03:20 |
| morganfainberg | and it doesn't make intuitive sense | 03:21 |
| ayoung-zzz | recursive would be much clearer. I think I'll bring back the old code. | 03:21 |
| morganfainberg | i'm also not sure that there is a performance win to not just building the tree and then doing the leaf for loop | 03:21 |
| morganfainberg | it seems like we're doing worst case O(2n) vs a guaranteed o(n+1) | 03:22 |
| ayoung-zzz | now, he took the key thing from me, and I am not certain it makes sense,, the key being user='asikjdhflkjasd' | 03:22 |
| morganfainberg | i might be wrong though. | 03:22 |
| ayoung-zzz | or user='*' | 03:22 |
| ayoung-zzz | OK...gonna crash. This can wait | 03:23 |
| *** ayoung-zzz is now known as ayoung-zzzZZ | 03:23 | |
| morganfainberg | sure | 03:23 |
| morganfainberg | plenty of time to discuss it | 03:23 |
| morganfainberg | night dude | 03:23 |
| *** kfox1111 has joined #openstack-keystone | 04:10 | |
| *** kfox1111 has quit IRC | 04:22 | |
| *** thedodd has quit IRC | 04:25 | |
| *** dims has quit IRC | 04:59 | |
| *** dims has joined #openstack-keystone | 05:00 | |
| *** chandan_kumar has joined #openstack-keystone | 05:14 | |
| openstackgerrit | Jenkins proposed a change to openstack/keystone: Imported Translations from Transifex https://review.openstack.org/78525 | 06:00 |
| *** chandan_kumar has quit IRC | 06:34 | |
| *** dstanek has joined #openstack-keystone | 06:49 | |
| openstackgerrit | Clint "SpamapS" Byrum proposed a change to openstack/keystone: Discourage use of pki_setup https://review.openstack.org/80819 | 07:05 |
| *** flaper87|afk is now known as flaper87 | 09:17 | |
| *** dstanek has quit IRC | 09:20 | |
| *** henrynash has joined #openstack-keystone | 10:28 | |
| *** morganfainberg is now known as morganfainberg_Z | 11:12 | |
| *** henrynash has quit IRC | 11:29 | |
| *** henrynash has joined #openstack-keystone | 11:34 | |
| openstackgerrit | Clint "SpamapS" Byrum proposed a change to openstack/keystone: Discourage use of pki_setup https://review.openstack.org/80819 | 12:12 |
| *** dims has quit IRC | 12:40 | |
| *** dims has joined #openstack-keystone | 12:45 | |
| *** lcostantino has joined #openstack-keystone | 13:20 | |
| *** lcostantino has quit IRC | 13:39 | |
| *** henrynash has quit IRC | 13:39 | |
| *** henrynash has joined #openstack-keystone | 13:42 | |
| *** henrynash has quit IRC | 13:48 | |
| *** zhiyan_ is now known as zhiyan | 15:39 | |
| *** thedodd has joined #openstack-keystone | 15:42 | |
| *** zhiyan is now known as zhiyan_ | 15:46 | |
| *** thedodd has quit IRC | 16:41 | |
| *** wchrisj has joined #openstack-keystone | 17:09 | |
| *** pete5 has joined #openstack-keystone | 17:30 | |
| *** wchrisj has quit IRC | 17:39 | |
| jogo | morganfainberg_Z: thanks stable/havana is working again | 20:01 |
| *** henrynash has joined #openstack-keystone | 20:41 | |
| *** leseb has joined #openstack-keystone | 20:46 | |
| *** bvandenh has quit IRC | 21:19 | |
| *** bvandenh has joined #openstack-keystone | 21:21 | |
| *** leseb has quit IRC | 22:03 | |
| *** thiagop_ has quit IRC | 22:43 | |
| *** harlowja_away has quit IRC | 22:43 | |
| *** zhiyan_ has quit IRC | 22:43 | |
| *** Daviey has quit IRC | 22:43 | |
| *** YorikSar has quit IRC | 22:43 | |
| *** sudorandom has quit IRC | 22:43 | |
| *** marekd|away has quit IRC | 22:43 | |
| *** bknudson has quit IRC | 22:43 | |
| *** chmouel has quit IRC | 22:43 | |
| *** bvandenh has quit IRC | 22:43 | |
| *** pete5 has quit IRC | 22:43 | |
| *** dolphm has quit IRC | 22:43 | |
| *** lbragstad has quit IRC | 22:43 | |
| *** haneef_ has quit IRC | 22:43 | |
| *** henrynash has quit IRC | 22:43 | |
| *** dims has quit IRC | 22:43 | |
| *** jimbaker has quit IRC | 22:43 | |
| *** dvorak has quit IRC | 22:43 | |
| *** mberlin1 has quit IRC | 22:43 | |
| *** openstackgerrit has quit IRC | 22:43 | |
| *** jaypipes has quit IRC | 22:43 | |
| *** jamielennox|away has quit IRC | 22:43 | |
| *** flaper87 has quit IRC | 22:43 | |
| *** koolhead17 has quit IRC | 22:43 | |
| *** jraim has quit IRC | 22:43 | |
| *** jogo has quit IRC | 22:43 | |
| *** dtroyer has quit IRC | 22:43 | |
| *** jordant has quit IRC | 22:43 | |
| *** zigo has quit IRC | 22:43 | |
| *** mhu has quit IRC | 22:43 | |
| *** mfisch has quit IRC | 22:43 | |
| *** ChanServ has quit IRC | 22:43 | |
| *** morganfainberg_Z has quit IRC | 22:43 | |
| *** vhoward has quit IRC | 22:43 | |
| *** huats has quit IRC | 22:43 | |
| *** ayoung-zzzZZ has quit IRC | 22:43 | |
| *** anteaya has quit IRC | 22:43 | |
| *** luisbg has quit IRC | 22:43 | |
| *** bvandenh has joined #openstack-keystone | 22:45 | |
| *** henrynash has joined #openstack-keystone | 22:45 | |
| *** pete5 has joined #openstack-keystone | 22:45 | |
| *** dims has joined #openstack-keystone | 22:45 | |
| *** mberlin1 has joined #openstack-keystone | 22:45 | |
| *** openstackgerrit has joined #openstack-keystone | 22:45 | |
| *** jraim has joined #openstack-keystone | 22:45 | |
| *** zhiyan_ has joined #openstack-keystone | 22:45 | |
| *** harlowja_away has joined #openstack-keystone | 22:45 | |
| *** vhoward has joined #openstack-keystone | 22:45 | |
| *** jogo has joined #openstack-keystone | 22:45 | |
| *** dolphm has joined #openstack-keystone | 22:45 | |
| *** Daviey has joined #openstack-keystone | 22:45 | |
| *** YorikSar has joined #openstack-keystone | 22:45 | |
| *** sudorandom has joined #openstack-keystone | 22:45 | |
| *** jimbaker has joined #openstack-keystone | 22:45 | |
| *** dvorak has joined #openstack-keystone | 22:45 | |
| *** marekd|away has joined #openstack-keystone | 22:45 | |
| *** lbragstad has joined #openstack-keystone | 22:45 | |
| *** haneef_ has joined #openstack-keystone | 22:45 | |
| *** flaper87 has joined #openstack-keystone | 22:45 | |
| *** bknudson has joined #openstack-keystone | 22:45 | |
| *** jordant has joined #openstack-keystone | 22:45 | |
| *** zigo has joined #openstack-keystone | 22:45 | |
| *** huats has joined #openstack-keystone | 22:45 | |
| *** mhu has joined #openstack-keystone | 22:45 | |
| *** ayoung-zzzZZ has joined #openstack-keystone | 22:45 | |
| *** chmouel has joined #openstack-keystone | 22:45 | |
| *** mfisch has joined #openstack-keystone | 22:45 | |
| *** jaypipes has joined #openstack-keystone | 22:45 | |
| *** jamielennox|away has joined #openstack-keystone | 22:45 | |
| *** koolhead17 has joined #openstack-keystone | 22:45 | |
| *** luisbg has joined #openstack-keystone | 22:45 | |
| *** anteaya has joined #openstack-keystone | 22:45 | |
| *** dtroyer has joined #openstack-keystone | 22:45 | |
| *** morganfainberg_Z has joined #openstack-keystone | 22:45 | |
| *** ChanServ has joined #openstack-keystone | 22:45 | |
| *** dickson.freenode.net sets mode: +o ChanServ | 22:45 | |
| *** flaper87 is now known as flaper87|afk | 22:45 | |
| *** thiagop_ has joined #openstack-keystone | 22:56 | |
| *** leseb has joined #openstack-keystone | 23:14 | |
| *** jamielennox|away is now known as jamielennox | 23:16 | |
| *** leseb has quit IRC | 23:18 | |
| *** henrynash has quit IRC | 23:21 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!