| dstanek | jamielennox: waiting on stuff to be reviewed? | 00:01 |
|---|---|---|
| *** marcoemorais has joined #openstack-keystone | 00:02 | |
| morganfainberg | dstanek, ok i know you have memory-related issues for running tests | 00:06 |
| morganfainberg | dstanek, what would your opinion be of moving to an in-memory sqlite db? | 00:06 |
| *** gokrokve_ has quit IRC | 00:13 | |
| *** gokrokve has joined #openstack-keystone | 00:14 | |
| dstanek | morganfainberg: i think that would be fine - i don't really have memory issues anymore | 00:18 |
| *** gokrokve has quit IRC | 00:19 | |
| morganfainberg | dstanek, i'll see if I can make that work sdague just pointed out that is what nova did to help limit the issues with needing tmpfs | 00:19 |
| dstanek | morganfainberg: i'm experimenting with some stuff to speed up the tests - they should not be this insanely slow | 00:25 |
| morganfainberg | dstanek, i'm looking at it as well. | 00:25 |
| dstanek | all tests in < 60 seconds | 00:28 |
| dstanek | i'm at 1479 seconds now | 00:31 |
| openstackgerrit | Brant Knudson proposed a change to openstack/keystone: Configurable token hash algorithm https://review.openstack.org/80401 | 00:31 |
| morganfainberg | dstanek,dstanek , so... | 00:32 |
| morganfainberg | dstanek, i think i know what jumped the time | 00:33 |
| dstanek | notifications or revocation would be my guess | 00:33 |
| morganfainberg | dstanek, it's revoke_api, once it's loaded the dependency injection always occurs. | 00:33 |
| dstanek | really? how? | 00:34 |
| morganfainberg | dstanek, same thing with any @dependency.optional | 00:34 |
| morganfainberg | i'm checking but... | 00:34 |
| dstanek | ah, i see if will always be created since it thinks it should | 00:34 |
| morganfainberg | yep | 00:34 |
| dstanek | we shouldn't be using DI in tests anyway | 00:34 |
| morganfainberg | we don't clear the dep registry (to be fair, how could we?) | 00:34 |
| morganfainberg | except we need DI since we do restful tests | 00:35 |
| bknudson | Ran 3163 (+527) tests in 125.612s (-197.584s) | 00:35 |
| morganfainberg | bknudson, using parallel test pathc? | 00:35 |
| morganfainberg | bknudson, or normal? | 00:35 |
| morganfainberg | bknudson, and w/ or w/o tmpfs? | 00:35 |
| bknudson | morganfainberg: this is with 65870 | 00:35 |
| dstanek | bknudson: you must be running a monster node compared to what i run | 00:35 |
| morganfainberg | bknudson, yeah, it really helps | 00:36 |
| bknudson | dstanek: I just got this new system... gave the vm 6 cpus | 00:36 |
| bknudson | I don't think I've seen more than 2 cpus used before this. | 00:36 |
| morganfainberg | i'm usually running 4 vcpus, i think thats what jenkins uses. | 00:36 |
| dstanek | also i would love to see a way to use pyopenssl or some C binding to sign/verify tokens | 00:37 |
| dstanek | using subprocess like we are doesn't seem like the most scalable solution | 00:37 |
| morganfainberg | dstanek, but basically, our DI is naive | 00:37 |
| morganfainberg | dstanek, once you import the dep, it will always be used. | 00:37 |
| morganfainberg | optional or not | 00:38 |
| dstanek | morganfainberg: i started to guice it up - maybe i'll keep going | 00:38 |
| morganfainberg | it hurts us a lot w/ testing | 00:38 |
| dstanek | but DI should only be used in the tests that create the servers | 00:38 |
| morganfainberg | dstanek, it should be used only in the tests that create the server AND where we want to test that injected system | 00:38 |
| bknudson | and yes I am using tmpfs. | 00:38 |
| morganfainberg | dstanek, the bulk of our slowness is restful tests | 00:39 |
| morganfainberg | bknudson, cool. | 00:39 |
| dstanek | morganfainberg: yeah, that's what i'm working on now | 00:39 |
| morganfainberg | dstanek, i think that will bring us back down to (non parallel) ~380-500s test time | 00:39 |
| bknudson | we have way too many tests that think they need to exercise the rest stck | 00:39 |
| openstackgerrit | David Stanek proposed a change to openstack/keystone: Uses generator expressions instead of filter https://review.openstack.org/70816 | 00:39 |
| bknudson | we should have one set of tests to cover the rest translation | 00:40 |
| bknudson | the rest can go to the controller | 00:40 |
| morganfainberg | bknudson, that wouldn't solve our actual issues | 00:40 |
| morganfainberg | bknudson, but it would be a step in the right direciton | 00:40 |
| morganfainberg | in theory we could move most of those rest tests to tempest | 00:41 |
| morganfainberg | it's kinda what tempest is meant for | 00:41 |
| bknudson | they don't want them | 00:41 |
| bknudson | I tried | 00:41 |
| morganfainberg | i thought the didn't want the ksc ones? | 00:41 |
| dstanek | i think we should have some in our tree as well - otherwise i'd have to run tempest all the time | 00:42 |
| morganfainberg | dstanek, hehe | 00:42 |
| morganfainberg | dstanek, i'd rather not have to do that either | 00:42 |
| bknudson | right, they don't want the ksc ones. | 00:42 |
| morganfainberg | bknudson, i was talking about the v3_auth et al ones that are really testing the rest stack | 00:43 |
| morganfainberg | our slowness is mostly on tests based on the restful test case | 00:43 |
| morganfainberg | ignoring the revoke tests atm | 00:43 |
| bknudson | morganfainberg: I think they would take tests like those... I don't think we want to trust that keystone tests are in tempest. | 00:44 |
| bknudson | tempest is happy to go their own way and remove tests that you think are important. | 00:45 |
| morganfainberg | bknudson, fair enough | 00:45 |
| morganfainberg | dstanek, actually, if we made all of the tests just set revoke_api to None explicitly :P it would probably speed stuff up | 00:46 |
| morganfainberg | dstanek, not saying that is a good plan | 00:47 |
| *** devlaps has quit IRC | 00:47 | |
| dstanek | morganfainberg: can you unset whatever registry holds the deps after each test? and then in test_v3_os_revoke make sure it's in there in it's setUp? | 00:47 |
| morganfainberg | dstanek, well except that we only populate that registry on import | 00:47 |
| bknudson | the registry should be reset after each test | 00:48 |
| morganfainberg | bknudson, we'd need to re-work DI then | 00:48 |
| dstanek | morganfainberg: also test_revoke would have to create it's own revoke_api | 00:48 |
| morganfainberg | bknudson, i'm not opposed to it. | 00:48 |
| dstanek | morganfainberg: can we just keep track of the ones listed as optional and only clear those? | 00:48 |
| bknudson | http://git.openstack.org/cgit/openstack/keystone/tree/keystone/tests/core.py#n439 | 00:48 |
| bknudson | or are you talking about something else? | 00:49 |
| morganfainberg | bknudson, huh ok i'm wrong then | 00:49 |
| bknudson | I don't understand putting @dependency.optional('revoke_api') on the test classes?? | 00:49 |
| morganfainberg | so reimport re-runs the decoprator? | 00:49 |
| bknudson | seems weird. | 00:49 |
| bknudson | not really the way I was thinking it would be used, but if it works. | 00:50 |
| bknudson | *shrug* | 00:50 |
| morganfainberg | bknudson, i don't understand using DI on any test cases tbh | 00:50 |
| dstanek | you're making me want to continue my port to the snakeguice model | 00:51 |
| bknudson | dstanek: anything is better than what we've got. | 00:51 |
| morganfainberg | dstanek, agreed | 00:51 |
| morganfainberg | bknudson, even | 00:51 |
| morganfainberg | ooh i see i misread this | 00:52 |
| morganfainberg | oh gah, | 00:52 |
| morganfainberg | bknudson, /me is not a huge fan of our DI | 00:53 |
| openstackgerrit | Brant Knudson proposed a change to openstack/keystone: Configurable token hash algorithm https://review.openstack.org/80401 | 00:53 |
| morganfainberg | ok so factories is what isn't cleared out. i see now | 00:54 |
| dstanek | i want more tests to be like the class i added here - https://review.openstack.org/#/c/81528/1/keystone/tests/test_catalog.py | 00:54 |
| *** andreaf has quit IRC | 00:54 | |
| dstanek | i needed a manager, so i created one in setUp - no magic | 00:55 |
| openstackgerrit | David Stanek proposed a change to openstack/keystone: More notification unit tests https://review.openstack.org/81659 | 00:55 |
| openstackgerrit | David Stanek proposed a change to openstack/keystone: Refactor notifications https://review.openstack.org/81660 | 00:55 |
| openstackgerrit | David Stanek proposed a change to openstack/keystone: wip: Implements notifications with blinker https://review.openstack.org/81661 | 00:55 |
| jamielennox | morganfainberg, bknudson: https://review.openstack.org/#/c/77026/ <- the @positional decorator | 00:56 |
| jamielennox | dstanek and others feel free as well ^ | 00:56 |
| dstanek | i eventually want a base class for database related tests - i just have to see what's wrong with my port to oslotests | 00:56 |
| jamielennox | dstanek: have you seen the oslo-incubator db test work? | 00:56 |
| morganfainberg | bknudson, actually i think we do always load anything in factories in tests | 00:56 |
| morganfainberg | bknudson, http://git.openstack.org/cgit/openstack/keystone/tree/keystone/common/dependency.py#n240 | 00:56 |
| dstanek | jamielennox: no | 00:56 |
| morganfainberg | we call resolve_future_dependencies in load_backends() explicitly w/o a provider | 00:56 |
| bknudson | morganfainberg: the factory thing is new... | 00:57 |
| morganfainberg | which appears would inject things like revoke_api in all cases | 00:57 |
| dstanek | morganfainberg: i know i'm missing the point, but the .copy caught my eye | 00:57 |
| jamielennox | dstanek: https://github.com/openstack/oslo-incubator/blob/master/openstack/common/db/sqlalchemy/test_base.py | 00:57 |
| bknudson | morganfainberg: reset() isn't clearing _factories like it should. | 00:57 |
| morganfainberg | dstanek, lol not changing the iterator out from under | 00:58 |
| jamielennox | i found it when setting up db testing for kite - i can't find anyone using it though because there does seem to be some problems with it | 00:58 |
| bknudson | jamielennox: that seems to be a common theme with oslo libs. | 00:58 |
| bknudson | you try to use it and it's got problems. | 00:58 |
| jamielennox | bknudson: i've seen that and i've no idea how - i thought it had to be working in another project before going to oslo | 00:59 |
| dstanek | jamielennox: that's pretty interesting | 00:59 |
| bknudson | jamielennox: once it gets into olso it's open season on changing it... and they obviously don't test it with the projects. | 00:59 |
| morganfainberg | bknudson, yeah it's new. it solved the revoke_api being "optional" or not | 00:59 |
| jamielennox | bknudson: i'd also appreciate if you could look at: https://review.openstack.org/#/c/60752/ it's been around a while now and it's important for progressing the session stuff | 01:02 |
| morganfainberg | bknudson, dstanek, i'm going to take a swing and cleaning up the DI stuff for Icehouse | 01:03 |
| morganfainberg | since i think a far reaching refactor wont work | 01:03 |
| morganfainberg | but at least we can make optionals not always inject w/ tests | 01:03 |
| bknudson | jamielennox: I hope to be able to get back to doing reviews soon... people who pay my bills are asking me to look at things lately. | 01:03 |
| dstanek | morganfainberg: sounds good to me | 01:04 |
| jamielennox | bknudson: no worries, i'm just starting to resort to bullying tactics :) | 01:04 |
| jamielennox | gyee: you here? i've got a new one you will be interested in too | 01:05 |
| *** leseb has joined #openstack-keystone | 01:05 | |
| *** amcrn has quit IRC | 01:06 | |
| *** leseb has quit IRC | 01:10 | |
| gyee | jamielennox, here | 01:34 |
| *** stevemar has joined #openstack-keystone | 01:34 | |
| jamielennox | gyee: that's ok, nothing urgent - i was talking to dtroyer and came up with: https://review.openstack.org/#/c/79542/ which i thought you'd be interested in | 01:35 |
| gyee | jamielennox, you only corrected the syntax stuff since the last patch right | 01:35 |
| gyee | https://review.openstack.org/#/c/60752/ I mean | 01:35 |
| jamielennox | gyee: yea, i think i answered the other questions | 01:35 |
| bknudson | jamielennox: you figured out stevedore? | 01:35 |
| jamielennox | bknudson: i'm not sure i figured it out | 01:36 |
| jamielennox | bknudson: but enough to get it to do what i needed | 01:36 |
| bknudson | jamielennox: I assume it finds and imports things? | 01:36 |
| jamielennox | bknudson: yea, its a nicer wrapper around dealing with setuptools entry points on your own | 01:37 |
| jamielennox | which isn't that hard just a bit messy | 01:37 |
| openstackgerrit | Brant Knudson proposed a change to openstack/python-keystoneclient: Allow hash tokens with sha256 https://review.openstack.org/80398 | 01:38 |
| gyee | jamielennox, so you load the plugin from the entry points | 01:39 |
| jamielennox | gyee: yep | 01:39 |
| jamielennox | gyee: i think it will be more useful when we have non-versioned auth plugins like:https://review.openstack.org/#/c/81147/ | 01:40 |
| *** stevemar has quit IRC | 01:40 | |
| jamielennox | so you have a v2password and a v3password which will take the versioned endpoints, or you just have password that most people will use and does version discovery first | 01:40 |
| jamielennox | and then just offload to the v2 or v3 depending on what it finds | 01:41 |
| gyee | jamielennox, how does it know about the custom plugins? | 01:41 |
| jamielennox | gyee: setuptools entry points | 01:41 |
| jamielennox | anything that defines a plugin entry point like: https://review.openstack.org/#/c/79542/4/setup.cfg is fair game | 01:42 |
| gyee | so I have a custom plugin in a custom package | 01:42 |
| gyee | I just need to overwrite keystoneclient.auth.plugin entry point | 01:42 |
| gyee | ? | 01:42 |
| jamielennox | gyee: it's not a case of overwrite | 01:43 |
| jamielennox | if you define an entrypoint with keysteonclient.auth.plugin then that will be available to keystoeclient | 01:43 |
| jamielennox | but i'm working on the principal that all auth plugins MUST be specified by name | 01:44 |
| jamielennox | so if you want to load the password plugin you will have to specify something like --os-auth password --params --for --password | 01:44 |
| jamielennox | i don't want to be in the situation of guessing the most appropriate plugin based on arguments | 01:44 |
| jamielennox | but if you do kerberos out of tree and you say --os-auth kerberos then it will use the plugin from your custom package | 01:45 |
| gyee | jamielennox, I am not setuptools savvy, here's a stupid question. Say I install both python-keystoneclient and a custom package, but define keystoneclient.auth.plugin in entry point section, keystoneclient will know about all of them? | 01:47 |
| jamielennox | gyee: yes | 01:47 |
| gyee | sweeet! | 01:47 |
| jamielennox | it will essentially append it to the list | 01:48 |
| gyee | love it! | 01:48 |
| * gyee needs to ramp up on stevedore | 01:49 | |
| jamielennox | so i'm not sure i've gotten everything about the AuthParams correct but that's the plugin descriptors you were talking about previously | 01:49 |
| *** chandan_kumar has joined #openstack-keystone | 01:53 | |
| *** shakamunyi has joined #openstack-keystone | 02:00 | |
| *** marcoemorais has quit IRC | 02:06 | |
| *** marcoemorais has joined #openstack-keystone | 02:06 | |
| *** stevemar has joined #openstack-keystone | 02:07 | |
| *** lbragstad has joined #openstack-keystone | 02:11 | |
| morganfainberg | env | 02:14 |
| *** hill has joined #openstack-keystone | 02:15 | |
| *** hill is now known as Guest71722 | 02:15 | |
| *** richm has quit IRC | 02:18 | |
| *** gyee has quit IRC | 02:20 | |
| ayoung | jamielennox, morganfainberg so my wife suggested I use Latex to do this upcoming presentation I have on Keystone to our Global Support Services people....and It flipping rocks | 02:20 |
| morganfainberg | ayoung, LaTeX? | 02:21 |
| jamielennox | ayoung: i've found it variable | 02:21 |
| ayoung | Specifcially, I've been using it for diagrams, and I feel like I am finally capturing concisely some of the key points | 02:21 |
| ayoung | jamielennox, I found that there is a decent graphics support library, and UML on top of that | 02:21 |
| jamielennox | ayoung: i would still use it to compose pdfs, but i tried the presentation layer (can't remember what it's called) and it wasn't that useful | 02:21 |
| ayoung | jamielennox, using it to make a PDF to use for a slide show | 02:22 |
| jamielennox | but once you recognise the format everynow and then you will see someone using the latex defaults | 02:22 |
| jamielennox | morganfainberg: it's essentially a text layout language for making good PDFs and dvis etc | 02:23 |
| jamielennox | morganfainberg: we got taught it at uni, that if you want to write something serious then you should do it in latex | 02:23 |
| ayoung | jamielennox, morganfainberg http://admiyo.fedorapeople.org/openstack/keystone/keystone.pdf | 02:23 |
| morganfainberg | jamielennox, yeah. haven't really had an excuse to use it | 02:23 |
| ayoung | morganfainberg, so I would say it would have been a toss up between Latex and something HTML based if my wife hadn't just been through the process to make a poster | 02:24 |
| jamielennox | ayoung: nkinder's most recent pres i saw was in that gnome slides program | 02:24 |
| jamielennox | pinpoint i think it was | 02:24 |
| jamielennox | that looks useful for much the same reasons, all in text | 02:24 |
| jamielennox | but it had the advantage of knowing how to use a presenters screen as well | 02:24 |
| ayoung | jamielennox, the thing I really like is that I am working text and compiling, | 02:25 |
| ayoung | git and everything | 02:25 |
| ayoung | http://admiyo.fedorapeople.org/openstack/keystone/keystone.tex | 02:26 |
| ayoung | it is pretty easy to understand once you look at it, and I like the end results | 02:26 |
| ayoung | thing is, there is a decent poster layout that uses the same library, I was thinkg that maybe I'll do some "keystone internals" as a poster and we can use as a visual during the summit based on these graphics | 02:26 |
| ayoung | BTW...that really should be stamped "DRAFT" | 02:27 |
| jamielennox | ayoung: interesting i haven't used the uml package | 02:27 |
| ayoung | jamielennox, funny thing, for interfaces to do <<this>> it needs \\guillemonts which is not in the codeset I am using...so I had to not use that | 02:27 |
| ayoung | I'm sure I can figure out how to get it in, but I really don't want to use the [french] encoding if I don't have to | 02:28 |
| ayoung | anyways, just wanted to share, and let you guys know why I 've been AWOL today...and now I am headed to bed | 02:28 |
| ayoung | jamielennox, he did a nice job with it. I'm doing pretty basic stuff, but some of the more complex examples look really good | 02:31 |
| ayoung | from http://perso.ensta-paristech.fr/~kielbasi/tikzuml/index.php?lang=en&id=doc#t-1 | 02:31 |
| *** marcoemorais has quit IRC | 02:31 | |
| ayoung | http://perso.ensta-paristech.fr/~kielbasi/tikzuml/index.php?lang=en&id=doc#t-5 | 02:31 |
| jamielennox | tool of the month/might be slightly longer now for me is httpie | 02:31 |
| jamielennox | sooo much better for testing than wget/curl | 02:32 |
| ayoung | interesting | 02:32 |
| jamielennox | i should have told people about that one before now, i've been using it for a while and can't go back now | 02:32 |
| ayoung | jamielennox, I'll give it a try, but some V3 API examples using it might be a worthwhile blog post | 02:33 |
| *** daneyon has joined #openstack-keystone | 02:43 | |
| *** marcoemorais has joined #openstack-keystone | 02:54 | |
| *** mberlin1 has joined #openstack-keystone | 02:56 | |
| *** mberlin has quit IRC | 02:58 | |
| *** Guest71722 has quit IRC | 03:00 | |
| *** Guest71722 has joined #openstack-keystone | 03:01 | |
| *** derek_c has quit IRC | 03:01 | |
| *** Guest71722_ has joined #openstack-keystone | 03:03 | |
| *** Guest71722 has quit IRC | 03:06 | |
| *** marcoemorais1 has joined #openstack-keystone | 03:15 | |
| *** marcoemorais has quit IRC | 03:15 | |
| *** marcoemorais1 has quit IRC | 03:15 | |
| *** Guest71722__ has joined #openstack-keystone | 03:19 | |
| *** Guest71722_ has quit IRC | 03:22 | |
| *** chandan_kumar has quit IRC | 03:22 | |
| *** harlowja is now known as harlowja_away | 03:23 | |
| *** gtt116 has joined #openstack-keystone | 03:25 | |
| *** Guest71722__ has quit IRC | 03:39 | |
| *** Guest71722__ has joined #openstack-keystone | 03:39 | |
| *** Guest71722__ has quit IRC | 03:44 | |
| *** david-lyle has joined #openstack-keystone | 03:50 | |
| openstackgerrit | Jamie Lennox proposed a change to openstack/python-keystoneclient: Add an exclude option to AuthToken middleware https://review.openstack.org/81695 | 03:55 |
| *** gokrokve has joined #openstack-keystone | 04:09 | |
| *** saju_m has joined #openstack-keystone | 04:12 | |
| nkinder | jamielennox: yeah, it was in pinpoint | 04:23 |
| jamielennox | nkinder: i like the idea of doing things in text - i've gotten good things out of LaTeX in the past but it's just a lot of work to get right | 04:24 |
| jamielennox | and generally the gnome dep doesn't make much difference to me | 04:25 |
| nkinder | yeah, haven't tried LaTeX myself | 04:25 |
| *** gokrokve has quit IRC | 04:49 | |
| *** stevemar has quit IRC | 04:53 | |
| *** gokrokve has joined #openstack-keystone | 04:55 | |
| *** derek_c has joined #openstack-keystone | 04:59 | |
| *** chandan_kumar has joined #openstack-keystone | 05:10 | |
| *** chandan_kumar has quit IRC | 05:17 | |
| *** amcrn has joined #openstack-keystone | 05:31 | |
| *** gokrokve_ has joined #openstack-keystone | 05:34 | |
| *** gokrokve has quit IRC | 05:38 | |
| *** daneyon has quit IRC | 05:41 | |
| *** gokrokve_ has quit IRC | 05:57 | |
| openstackgerrit | Jenkins proposed a change to openstack/keystone: Imported Translations from Transifex https://review.openstack.org/78525 | 06:00 |
| *** saju_m has quit IRC | 06:02 | |
| *** saju_m has joined #openstack-keystone | 06:04 | |
| *** saju_m has quit IRC | 06:07 | |
| morganfainberg | wow, we have some serious cruft in our code. | 06:08 |
| *** saju_m has joined #openstack-keystone | 06:08 | |
| *** saju_m has quit IRC | 06:35 | |
| *** wchrisj has quit IRC | 06:54 | |
| *** bvandenh has joined #openstack-keystone | 06:59 | |
| *** bvandenh has quit IRC | 07:07 | |
| *** derek_c has quit IRC | 07:15 | |
| *** shakamunyi has quit IRC | 07:15 | |
| openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Remove extraenous instantiations of managers https://review.openstack.org/81720 | 07:22 |
| openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Rename keystone.tests.fixtures https://review.openstack.org/81721 | 07:26 |
| *** henrynash has joined #openstack-keystone | 07:27 | |
| morganfainberg | henrynash, allo | 07:29 |
| henrynash | morganfainberg: hi | 07:29 |
| morganfainberg | henrynash, how goes? | 07:29 |
| henrynash | morganfainberg: good, you? | 07:29 |
| *** bvandenh has joined #openstack-keystone | 07:29 | |
| morganfainberg | henrynash, not bad, just did some deep debugging on weird edge-case manager instantiations | 07:30 |
| henrynash | morganfainberg: just looking at those changes.... | 07:30 |
| morganfainberg | was getting different configurations / dependency injections depending on when things were run | 07:30 |
| morganfainberg | took me a bunch of time to figure out what was going on. feels good to chase something like this down | 07:30 |
| henrynash | morganfainberg: nice…you want to get these into rc1 ? | 07:32 |
| morganfainberg | henrynash, it would be nice to. but it isn't a hard requirement | 07:33 |
| morganfainberg | it will affect the next one in the chain which should help speed our tests back up (not as much as the parallel testing one) | 07:33 |
| henrynash | morganfainberg: any risks in doing so (I don't think so)? | 07:33 |
| morganfainberg | henrynash, unlikely | 07:33 |
| henrynash | morganfainberg: ok…nice job in work thing through something yukky like that….I'll review….(although seems pretty good from first glance) | 07:35 |
| morganfainberg | henrynash, hehe :) | 07:35 |
| *** henrynash has quit IRC | 07:37 | |
| *** dstanek has quit IRC | 07:38 | |
| *** shakamunyi has joined #openstack-keystone | 07:41 | |
| *** marekd|away is now known as marekd | 07:41 | |
| *** leseb has joined #openstack-keystone | 07:44 | |
| *** shakamunyi has quit IRC | 07:45 | |
| *** flaper87|afk is now known as flaper87 | 07:50 | |
| *** leseb has quit IRC | 07:52 | |
| *** andreaf has joined #openstack-keystone | 07:56 | |
| *** saju_m has joined #openstack-keystone | 07:57 | |
| openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Make optional dependencies in tests optional https://review.openstack.org/81732 | 08:20 |
| openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Remove extraenous instantiations of managers https://review.openstack.org/81720 | 08:20 |
| *** amcrn has quit IRC | 08:26 | |
| openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Remove extraenous instantiations of managers https://review.openstack.org/81720 | 08:29 |
| openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Make optional dependencies in tests optional https://review.openstack.org/81732 | 08:29 |
| *** shakamunyi has joined #openstack-keystone | 08:42 | |
| *** shakamunyi has quit IRC | 08:46 | |
| *** morganfainberg is now known as morganfainberg_Z | 08:54 | |
| *** leseb has joined #openstack-keystone | 08:58 | |
| *** henrynash has joined #openstack-keystone | 09:12 | |
| *** shakamunyi has joined #openstack-keystone | 09:43 | |
| *** shakamunyi has quit IRC | 09:47 | |
| *** rwsu has quit IRC | 10:18 | |
| *** henrynash has quit IRC | 10:31 | |
| *** rwsu has joined #openstack-keystone | 10:33 | |
| *** shakamunyi has joined #openstack-keystone | 10:43 | |
| *** shakamunyi has quit IRC | 10:48 | |
| *** zoresvit has joined #openstack-keystone | 10:49 | |
| zoresvit | Hello! In Keystone with LDAP backend, will user tokens be revoked once user gets locked/deleted in LDAP? | 10:50 |
| *** chandankumar_ has quit IRC | 11:00 | |
| *** chandankumar has joined #openstack-keystone | 11:01 | |
| *** leseb has quit IRC | 11:14 | |
| *** leseb has joined #openstack-keystone | 11:15 | |
| *** leseb has quit IRC | 11:19 | |
| *** shakamunyi has joined #openstack-keystone | 11:44 | |
| *** shakamunyi has quit IRC | 11:48 | |
| *** david-lyle has quit IRC | 11:52 | |
| *** henrynash has joined #openstack-keystone | 11:57 | |
| *** saju_m has quit IRC | 12:09 | |
| ayoung | dolphm, https://review.openstack.org/#/c/72126/ was made a while back | 12:17 |
| *** saju_m has joined #openstack-keystone | 12:27 | |
| dolphm | ayoung: a few nits, but lgtm | 12:29 |
| *** leseb has joined #openstack-keystone | 12:40 | |
| *** shakamunyi has joined #openstack-keystone | 12:45 | |
| *** shakamunyi has quit IRC | 12:49 | |
| *** zoresvit has quit IRC | 12:51 | |
| *** saju_m has quit IRC | 12:53 | |
| *** bknudson has quit IRC | 12:57 | |
| *** mberlin1 has quit IRC | 13:02 | |
| *** flaper87 is now known as flaper87|afk | 13:05 | |
| *** saju_m has joined #openstack-keystone | 13:07 | |
| *** flaper87|afk is now known as flaper87 | 13:12 | |
| *** mberlin has joined #openstack-keystone | 13:13 | |
| *** browne has joined #openstack-keystone | 13:15 | |
| ayoung | dolphm, do you want to keep the "revoke events slowdown" issue as an rc blocker? I am not even certain it is a real problem. Even if it does slow things down, with RE optional, it is not a real issue. | 13:20 |
| ayoung | I had made the changes to fix it...but must not have commited them to a branch or deleted the branch in error | 13:20 |
| ayoung | I think | 13:20 |
| ayoung | I know I discussed the right way to cached the revocation tree with morganfainberg_Z as a staight up dogpile KVS in memory | 13:21 |
| *** bknudson has joined #openstack-keystone | 13:21 | |
| dolphm | ayoung: i'd like to see a patch for it at least :-/ | 13:21 |
| ayoung | if it is important, I'll knock it out now | 13:21 |
| ayoung | OK...getting on it | 13:21 |
| *** vhoward has left #openstack-keystone | 13:21 | |
| ayoung | bknudson, question about https://review.openstack.org/#/c/47441/8/keystone/identity/backends/ldap.py | 13:21 |
| dolphm | bknudson: did you notice any performance improvement from https://review.openstack.org/#/c/65870/ ? | 13:22 |
| dolphm | morganfainberg_Z: ^ | 13:22 |
| ayoung | I think that I can make the change you are suggesting on line 217ish, if I always pop the DN...the reason it was like that was to reduce the code impact since it is only for identity..right now | 13:22 |
| *** saju_m has quit IRC | 13:23 | |
| bknudson | dolphm: Ran 3163 (+527) tests in 125.612s (-197.584s) | 13:23 |
| dolphm | bknudson: =( with tox or running testr --parallel directly? i assume it *shouldn't* matter | 13:23 |
| bknudson | dolphm: tox | 13:23 |
| bknudson | dolphm: it was using all the cpus... i've got 6 on this vm | 13:24 |
| dolphm | bknudson: i'm just running on my laptop, but it actually increases the overall runtime substantially with tox, and doesn't seem to have any effect with testr | 13:24 |
| bknudson | dolphm: for me with that change it took 2 mins where it used to take 5 mins. I could try it again | 13:25 |
| dolphm | bknudson: i'll run it a few more times as well -- i'm hoping the tox run was a fluke | 13:25 |
| dolphm | bknudson: it was definitely running in parallel and the test lists look sanely divided | 13:26 |
| bknudson | ayoung: It looks like it's trying to reimplement a function in the subclass... why doesn't it use regular overriding? | 13:27 |
| ayoung | bknudson, it needs to call the baseclass function. | 13:27 |
| ayoung | Is there a cleaner way to do that? | 13:27 |
| ayoung | bknudson, somethinkg like super.blah... | 13:27 |
| bknudson | super(UserApi, self)._ldap_res_to_model() | 13:27 |
| ayoung | ok...that is what I was looking for. I tried it back then, but I thjink I had the syntax wrong | 13:28 |
| *** shakamunyi has joined #openstack-keystone | 13:32 | |
| dolphm | ayoung: fwiw, super() is easier to use in py3 | 13:32 |
| *** dstanek has joined #openstack-keystone | 13:35 | |
| dolphm | bknudson: yay! second attempt with tox finished in about half the time of a normal run from this week | 13:35 |
| ayoung | dolphm, starting to think that, in Juno, we push for py33 as the baseline for development. We need to fix eventlet for that. | 13:37 |
| ayoung | then treat py27 as "backport" | 13:37 |
| ayoung | I mean, both will still be required for the gate, just as far as a coding norm | 13:38 |
| dolphm | ayoung: if we could run on py33 at all, you'd be able to develop with whichever | 13:38 |
| ayoung | dolphm, yep...its eventlet that is stopping that as far as I know. | 13:38 |
| dolphm | ayoung: i think that's just the biggest blocker, not the last | 13:39 |
| *** saju_m has joined #openstack-keystone | 13:39 | |
| ayoung | probably truw | 13:39 |
| ayoung | true | 13:39 |
| dstanek | ayoung: there are 3 or 4 things that don't install on py3 the last time i checked | 13:39 |
| ayoung | maybe Juno 1 is the "make Keystone run on py33" milestone | 13:40 |
| ayoung | is stevedore and using entry points for plugins/extensions something we want to pursue? | 13:42 |
| *** wchrisj has joined #openstack-keystone | 13:43 | |
| *** nkinder has quit IRC | 13:44 | |
| *** andreaf has quit IRC | 13:46 | |
| dstanek | i'm not sure the model of entry point for extensions works for us | 13:48 |
| dstanek | do we have a need to automatically discover extensions without them being explicitly configured? | 13:48 |
| *** saju_m has quit IRC | 13:48 | |
| *** stevemar has joined #openstack-keystone | 13:49 | |
| openstackgerrit | ayoung proposed a change to openstack/python-keystoneclient: Regions Management https://review.openstack.org/79096 | 14:04 |
| *** andreaf has joined #openstack-keystone | 14:05 | |
| dstanek | it's eventlet, sqlalchemy-migrate, pysqlite and python-memcached | 14:08 |
| dstanek | eventlet's python3 branch seems to install ok, but i haven't used it | 14:09 |
| dstanek | i'm not sure why we need pysqlite - i thought python has had builtin support for it for a while now | 14:10 |
| *** flaper87 is now known as flaper87|afk | 14:13 | |
| *** dstanek_afk has joined #openstack-keystone | 14:14 | |
| *** dstanek has quit IRC | 14:15 | |
| *** dstanek_afk is now known as dstanek | 14:15 | |
| ayoung | dstanek, full court press once we get Icehouse delivered | 14:26 |
| ayoung | we each take one and beat it into submission: I'll hit migrate, you hit eventlet and pysqlite, and morganfainberg_Z deals with python-memcached | 14:27 |
| ayoung | or I take eventlet and you take the two sql ones, probably makes more sense | 14:28 |
| dstanek | ayoung: i'd but up for whatever | 14:29 |
| ayoung | interesting turn of phrase there | 14:29 |
| dstanek | lol | 14:30 |
| dstanek | isn't someone getting rid of migrate already? | 14:30 |
| ayoung | dstanek, nah, we were going to move to a different tool, but we instead decided to adopt migrate | 14:31 |
| ayoung | Alembic | 14:31 |
| ayoung | how do I run just one test with tox | 14:31 |
| ayoung | tox -epy27 keystone.tests.test_backend_ldap.LDAPIdentityEnabledEmulation still seems to be doing discovery | 14:32 |
| stevemar | ayoung, drop the keystone.tests. part | 14:32 |
| *** nkinder has joined #openstack-keystone | 14:32 | |
| dstanek | ayoung: it will do discovery anyway | 14:32 |
| stevemar | whats the context of " I'll hit migrate, you hit eventlet and pysqlite ... " | 14:33 |
| ayoung | no way to bypass that? | 14:33 |
| ayoung | stevemar, py33 support | 14:33 |
| stevemar | ayoung, nice | 14:33 |
| ayoung | stevemar, post Icehouse | 14:33 |
| stevemar | of course | 14:33 |
| dstanek | ayoung: not sure if it's our setup or testr | 14:33 |
| *** derek_c has joined #openstack-keystone | 14:35 | |
| openstackgerrit | A change was merged to openstack/keystone: Store groups ids objects list in the OS-FEDERATION object. https://review.openstack.org/81277 | 14:44 |
| browne | question: v2 is being deprecated. so is there a BP for other services to switch to keystone v3? also does the keystoneclient middleware even support v3 yet? | 14:45 |
| lbragstad | browne: some of the other services are slowly moving towards v3, I know there have been mailing list threads on the subject.. | 14:47 |
| * lbragstad digs for a link | 14:47 | |
| browne | i see that keystoneclient middleware always acquires a v2 token. https://github.com/openstack/python-keystoneclient/blob/master/keystoneclient/middleware/auth_token.py#L785 | 14:48 |
| browne | dont' we need v3 support in the middleware first? | 14:48 |
| lbragstad | browne: http://lists.openstack.org/pipermail/openstack-dev/2014-March/030403.html | 14:50 |
| browne | lbragstad: thanks | 14:50 |
| lbragstad | browne: yep | 14:51 |
| *** leseb has quit IRC | 14:55 | |
| *** thedodd has joined #openstack-keystone | 15:02 | |
| *** leseb has joined #openstack-keystone | 15:10 | |
| *** dstanek has quit IRC | 15:12 | |
| *** david-lyle has joined #openstack-keystone | 15:16 | |
| *** dstanek has joined #openstack-keystone | 15:17 | |
| *** gyee has joined #openstack-keystone | 15:25 | |
| *** derek_c has quit IRC | 15:29 | |
| *** shakamunyi has quit IRC | 15:29 | |
| *** daneyon has joined #openstack-keystone | 15:37 | |
| openstackgerrit | Ilya Pekelny proposed a change to openstack/keystone: Sync test_migrations https://review.openstack.org/80618 | 15:40 |
| openstackgerrit | Ilya Pekelny proposed a change to openstack/keystone: Comparision of database models and migrations. https://review.openstack.org/80630 | 15:40 |
| dolphm | dstanek: parallel test workers just entered the gate :) | 15:42 |
| *** gokrokve has joined #openstack-keystone | 15:42 | |
| dstanek | dolphm: nice! | 15:43 |
| openstackgerrit | A change was merged to openstack/keystone: Ability to turn off ldap referral chasing https://review.openstack.org/78521 | 15:46 |
| *** devlaps has joined #openstack-keystone | 15:49 | |
| openstackgerrit | A change was merged to openstack/keystone: Add user_id when calling populate_roles_for_groups https://review.openstack.org/81587 | 15:53 |
| *** joesavak has joined #openstack-keystone | 16:05 | |
| *** shakamunyi has joined #openstack-keystone | 16:06 | |
| *** zoresvit has joined #openstack-keystone | 16:10 | |
| *** shakamunyi has quit IRC | 16:10 | |
| *** leseb has quit IRC | 16:12 | |
| *** zoresvit has quit IRC | 16:14 | |
| *** derek_c has joined #openstack-keystone | 16:16 | |
| *** raildo_ has quit IRC | 16:21 | |
| *** derek_c has quit IRC | 16:22 | |
| *** marekd is now known as marekd|bbl | 16:25 | |
| *** gokrokve_ has joined #openstack-keystone | 16:25 | |
| *** gokrokve has quit IRC | 16:27 | |
| *** marcoemorais has joined #openstack-keystone | 16:32 | |
| *** derek_c has joined #openstack-keystone | 16:34 | |
| dstanek | dolphm: right now i have several clones that i work out of so that as i wait for tests to finish in one i can work on other bugs/reviews in another | 16:36 |
| *** dims has quit IRC | 16:37 | |
| *** leseb has joined #openstack-keystone | 16:43 | |
| *** leseb has quit IRC | 16:44 | |
| *** leseb has joined #openstack-keystone | 16:44 | |
| dolphm | dstanek: good move :P | 16:45 |
| *** leseb has quit IRC | 16:47 | |
| *** leseb has joined #openstack-keystone | 16:47 | |
| *** dims has joined #openstack-keystone | 16:49 | |
| openstackgerrit | Ilya Pekelny proposed a change to openstack/keystone: Sync test_migrations https://review.openstack.org/80618 | 16:56 |
| openstackgerrit | Ilya Pekelny proposed a change to openstack/keystone: Comparision of database models and migrations. https://review.openstack.org/80630 | 16:56 |
| *** leseb has quit IRC | 16:58 | |
| *** leseb has joined #openstack-keystone | 16:58 | |
| *** leseb has quit IRC | 17:02 | |
| *** derek_c has quit IRC | 17:14 | |
| dolphm | anyone seen this? looks like a critical transient (this happened in jenkins) http://pasteraw.com/l36dblfxt7vv5rcz9j6mgqkie1yi1r3 | 17:21 |
| dolphm | ayoung: ^ | 17:21 |
| ayoung | dolphm, I think it is an ABC ordering thing | 17:21 |
| ayoung | dolphm, when YorikSar and I were discussing the fixes he proposed, we saw something of that sort | 17:22 |
| dolphm | ayoung: abstract base class? | 17:22 |
| ayoung | no | 17:22 |
| ayoung | Alphabetical Order | 17:22 |
| ayoung | but...maybe noit | 17:22 |
| ayoung | that looks like a race condition | 17:22 |
| ayoung | dolphm, I am wondering if the comparison happens where one datetime is rounded and the other is not | 17:23 |
| ayoung | microseconds? | 17:23 |
| ayoung | dolphm, please file a bug on that...I am guessing it is something about the time allocations | 17:23 |
| *** leseb has joined #openstack-keystone | 17:23 | |
| *** morganfainberg_Z is now known as morganfainberg | 17:23 | |
| ayoung | it should be a problem only in unit tests if am right, but still needs to be fixed | 17:24 |
| dstanek | if if were a rounding issue i would expect it to happen a lot more | 17:24 |
| dolphm | ayoung: https://bugs.launchpad.net/keystone/+bug/1295261 | 17:24 |
| uvirtbot | Launchpad bug 1295261 in keystone "test_v3_os_revoke.OSRevokeTests: invalid event issued_before time; Too early" [Critical,New] | 17:24 |
| ayoung | dstanek, not if it was two different values rounded, and this only happens when one is rounded up and the other rounded down | 17:24 |
| morganfainberg | dolphm, did you remove your .testrepository directory? | 17:24 |
| ayoung | or something on a chaotic fringe like that | 17:24 |
| morganfainberg | dolphm, i was having the same issue because i had one w/ old data | 17:25 |
| morganfainberg | caused dumb binning of tests | 17:25 |
| dolphm | morganfainberg: this wasn't local... | 17:25 |
| morganfainberg | dolphm, oh | 17:26 |
| morganfainberg | dolphm, huh | 17:26 |
| dolphm | morganfainberg: i swear i saw that in one of the jenkins run for one of your patches? | 17:26 |
| dolphm | morganfainberg: i copied the logs and accidentally closed the jenkins window | 17:26 |
| morganfainberg | dolphm, i was seeing a net improvement, ~10min total, 380s vs some other similar runs 1000s+ | 17:26 |
| dstanek | ayoung: you may be right - before_time isn't normalized | 17:26 |
| dstanek | but i don't know what normalize is really doing | 17:26 |
| ayoung | dstanek, normalize is adding in the TZ data. | 17:27 |
| dstanek | ayoung: maybe it fails based on the timezone of the box running the tests then :-) | 17:27 |
| ayoung | dstanek, shouldn't | 17:28 |
| ayoung | dstanek, but keeping out time functions straight is a PITA | 17:28 |
| ayoung | the JSON marshalling choses one thing and the database another | 17:28 |
| * ayoung just ranting without confirming details | 17:28 | |
| morganfainberg | dolphm, example http://logs.openstack.org/99/81499/1/check/gate-keystone-python27/904e5a5/console.html#_2014-03-19_16_18_46_308 vs http://logs.openstack.org/70/65870/8/check/gate-keystone-python27/4a329c9/console.html#_2014-03-19_23_36_51_066 (is typically what i was seeing) | 17:28 |
| *** richm has joined #openstack-keystone | 17:29 | |
| dolphm | morganfainberg: my run time went down from 18-20 minutes to 13 minutes | 17:30 |
| morganfainberg | dolphm, that seems about normal. | 17:31 |
| morganfainberg | dolphm, gate is seeing 24min -> ~10-11 | 17:31 |
| dolphm | morganfainberg: nice! | 17:31 |
| morganfainberg | dolphm, next we should move to in-mem sqlite | 17:31 |
| morganfainberg | dolphm, like nova did, that means no more needing to specify TMPFS dir to get speedup | 17:32 |
| dolphm | ayoung: dstanek: morganfainberg: logstash query for the "too early" but https://review.openstack.org/#/c/81855/ | 17:32 |
| dolphm | bug* | 17:32 |
| morganfainberg | dolphm, ++ | 17:32 |
| ayoung | ++ | 17:32 |
| dolphm | shows 3 hits in last 48 hours | 17:33 |
| ayoung | ugh | 17:33 |
| morganfainberg | ayoung, https://review.openstack.org/#/c/81732/ in your fix for dependency you made it so optionals weren't optional if they had ever been imported | 17:33 |
| morganfainberg | ayoung, i'm working on fixing the current bug. | 17:33 |
| morganfainberg | ayoung, erm the check fail | 17:34 |
| morganfainberg | ayoung, it also looks like you never instantiated revoke.Manager() like you did w/ oauth so adding the pipeline wouldn't have enabled it | 17:34 |
| ayoung | morganfainberg, Um...that looks wrong | 17:34 |
| ayoung | morganfainberg, I think you misunderstand how this is supposed to work... | 17:35 |
| ayoung | the extensions themselves are not supposed to be added unless they are references | 17:35 |
| ayoung | referenced | 17:35 |
| morganfainberg | ayoung, you kept _factories around forever and if a provider wasn't instantiated you always instantiated it | 17:35 |
| ayoung | which, on a live server should only happen by the pipelie | 17:35 |
| ayoung | that is correct behavior | 17:35 |
| morganfainberg | ayoung, instantiating the manager should resolve the future deps | 17:36 |
| morganfainberg | ayoung, at least that is how it used to work | 17:36 |
| ayoung | morganfainberg, it only ends up in _factories if it gets registered | 17:36 |
| morganfainberg | ayoung, but if it was registered _ever_ all tests get it | 17:36 |
| morganfainberg | ayoung, forever | 17:36 |
| ayoung | and that is a problem because? | 17:36 |
| ayoung | wait... | 17:37 |
| morganfainberg | ayoung, test A registers revoke, test B doesn't need it | 17:37 |
| morganfainberg | test b gets it anyway | 17:37 |
| ayoung | no I see that | 17:37 |
| morganfainberg | and you registered it on import not on instantiation. | 17:37 |
| ayoung | ok, the right thing is to hace @dependecy.requires on the tests that need something, but our reset the backend logic doesn;t allow it | 17:37 |
| openstackgerrit | David Stanek proposed a change to openstack/keystone: Start using to oslotest https://review.openstack.org/79068 | 17:38 |
| openstackgerrit | David Stanek proposed a change to openstack/keystone: Allows override of stdout/stderr/log capturing https://review.openstack.org/79069 | 17:38 |
| ayoung | registering on import is the right approach | 17:38 |
| morganfainberg | ayoung, negative. | 17:38 |
| morganfainberg | ayoung, not with the current model | 17:38 |
| ayoung | explicit instantiation is how we got into this mess in the first place | 17:38 |
| ayoung | Ugh...OK, we were doing this anyway | 17:38 |
| ayoung | if you looked at the old patch, the other extension were explicitly added | 17:39 |
| morganfainberg | ayoung, yeah. lets commit to fixing this in Juno | 17:39 |
| * ayoung facepalm | 17:39 | |
| morganfainberg | ayoung, the issue is we can't rely on import if we can't clear that list, and we can't clear that list since import happens once. | 17:39 |
| ayoung | sure | 17:39 |
| ayoung | whatever... | 17:39 |
| morganfainberg | ayoung, we already said we wanted to overhaul DI. | 17:40 |
| morganfainberg | ayoung, and we should. dstanek has some work along those lines somewhere i think | 17:40 |
| ayoung | morganfainberg, is there a bug associated with https://review.openstack.org/#/c/81732/ ? | 17:40 |
| dolphm | morganfainberg: ++ | 17:41 |
| morganfainberg | ayoung, no i need to open one | 17:41 |
| ayoung | Ok, do that first... | 17:41 |
| morganfainberg | ayoung, i'll mark it as partial. | 17:41 |
| dstanek | morganfainberg, ayoung: yes, i've been toying with redoing di | 17:42 |
| ayoung | dstanek, take a look at that one and tell me if there is a real problem there...I'm not convinced | 17:43 |
| dstanek | i think the need for optional deps isn't related to how the di is working; i think it's a matter of not having a real model for extension | 17:43 |
| morganfainberg | dstanek, ++ | 17:43 |
| dolphm | dstanek: ++ | 17:44 |
| dstanek | an object should either depend on something or not | 17:44 |
| *** gokrokve has joined #openstack-keystone | 17:44 | |
| *** gokrokve_ has quit IRC | 17:44 | |
| dolphm | dstanek: we have a real model for extension (paste!), but we're unfortunately lazy about using it | 17:44 |
| morganfainberg | https://bugs.launchpad.net/keystone/+bug/1295280 | 17:44 |
| uvirtbot | Launchpad bug 1295280 in keystone "Dependency Injection always injects optional after import" [Undecided,New] | 17:44 |
| ayoung | morganfainberg, look at the patch before mine. I don't think there were any real "optional" dependencies in tests. | 17:45 |
| morganfainberg | ayoung, no there weren't. | 17:45 |
| dstanek | dolphm: i don't know that paste really gives you what you need in our code; paste give you the ability to automatically discover extensions, but i think we would still want people to manually configure them in most cases | 17:46 |
| dolphm | dstanek: i don't understand the "but"? | 17:46 |
| dstanek | i would want to make sure someone has to actively enable an extension. | 17:47 |
| dolphm | dstanek: right; paste gives you that | 17:48 |
| *** gokrokve has quit IRC | 17:50 | |
| dstanek | dolphm: i'm thinking at a deeper level in our code. for example, the token manager had to be updated for the revoke extension. | 17:50 |
| *** gokrokve has joined #openstack-keystone | 17:50 | |
| dstanek | it has an option dep on revoke_api which to me is weird. | 17:51 |
| dstanek | it also means that in many cases an "extension" can't be delivered by a third party without hacking keystone | 17:51 |
| dolphm | dstanek: that's the laziness i'm referring to -- we aren't using the pipeline very well | 17:52 |
| dolphm | dstanek: it's sort of half hearted -- we're using it for some endpoints, but not to modify others | 17:52 |
| dstanek | we may not actually have the need for this, but i don't think a pipeline would give you deep integration. it allow for decorator like actions at the higher level | 17:54 |
| dstanek | (not saying this is a good idea) but consider the power of sending defined signals where entities are about to be save and after they are saved. like django or mongoengine | 17:55 |
| *** gokrokve has quit IRC | 17:55 | |
| dstanek | or defining extention points where a extenstion can register callback that keystone will call | 17:55 |
| dstanek | lots of different way to architect it, but they key difference is that keystone doesn't have to changed to get deep integration | 17:56 |
| morganfainberg | dstanek, i always was a fan of hooks and registering hooks | 17:57 |
| morganfainberg | dstanek, but i don't know how scalable that actually is | 17:57 |
| dstanek | a practical example would be verifying a password meets certain criteria. having the right extension points available i could get notified before a password is saved and run arbitrary code against it | 17:58 |
| dstanek | morganfainberg: what wouldn't be scalable? | 17:59 |
| morganfainberg | dstanek, i've seen issues where the hooks get bound up because of poorly designed callbacks etc. | 17:59 |
| morganfainberg | dstanek, and/or odd interactions depending on when things are registered | 18:00 |
| morganfainberg | dstanek, but my experience was debugging coroutine C++ | 18:01 |
| dstanek | i can definitely see issues with plugins stomping on each other, but any model will likely have that problem | 18:01 |
| morganfainberg | dstanek, it was also a stupidly complex codebase | 18:02 |
| morganfainberg | dstanek, and near impossible to debug :P | 18:02 |
| dstanek | i don't think it is an issue as far as slowing the system down | 18:02 |
| morganfainberg | dstanek, well it did in this case. but it might have been a coroutine hell thing as well | 18:02 |
| dstanek | we're alread saying 'if opt_dep: do_stuff' - i just want the do_stuff somewhere more standard | 18:03 |
| morganfainberg | dstanek, true | 18:03 |
| dstanek | just food for thought | 18:04 |
| morganfainberg | dstanek, in either case, fixy next release :) | 18:04 |
| stevemar | dstanek, i wasn't a fan of how the token manager had to updated for revoke either | 18:08 |
| *** leseb has quit IRC | 18:08 | |
| dstanek | stevemar: i was just picking on that one because it was first in my grep - we do that in several places i think | 18:09 |
| morganfainberg | dstanek, oauth same thing. | 18:10 |
| stevemar | yep | 18:10 |
| dstanek | i think my grep found a bug: http://git.openstack.org/cgit/openstack/keystone/tree/keystone/common/models.py#n164 | 18:10 |
| stevemar | dstanek, very likely a bug... | 18:11 |
| ayoung | dstanek, the token provider dependency on the revoke_api would go away if it emitted events | 18:14 |
| ayoung | with the exception of checking revocation | 18:14 |
| ayoung | that should be done as a pipeline component | 18:15 |
| *** gokrokve has joined #openstack-keystone | 18:21 | |
| ayoung | so howdo I label a patch "proba bly should fix a bug but not sure? Partial?" | 18:26 |
| morganfainberg | ayoung, uhm. | 18:26 |
| morganfainberg | ayoung, i'd probably still use closes | 18:27 |
| ayoung | morganfainberg, k | 18:27 |
| morganfainberg | ayoung, if it isn't a partial fix, and should fix it, it closes the bug | 18:27 |
| ayoung | morganfainberg, what if it just makes it harder to reprodce | 18:27 |
| morganfainberg | ayoung, if it legitimately is just a partial fix and something more in-depth will be needed to really fix, partial would be correct | 18:28 |
| morganfainberg | ayoung, if it's just making it harder to happen, then probably partial | 18:28 |
| ayoung | morganfainberg, thing is, I don't know...I'll show you | 18:28 |
| morganfainberg | ayoung, k | 18:28 |
| openstackgerrit | ayoung proposed a change to openstack/keystone: Comparisons should account for instantaneous test execution https://review.openstack.org/81879 | 18:28 |
| ayoung | morganfainberg, ^^ | 18:28 |
| ayoung | morganfainberg, I should probably add in the times in failure message | 18:29 |
| morganfainberg | ayoung, sec | 18:29 |
| morganfainberg | ayoung, where is that value being written? | 18:30 |
| morganfainberg | the before_dime that is | 18:30 |
| morganfainberg | before_time ? | 18:30 |
| *** gokrokve has quit IRC | 18:31 | |
| morganfainberg | ayoung, oooh i think i see the root of the problem | 18:31 |
| morganfainberg | ayoung, give me a moment. | 18:31 |
| morganfainberg | ayoung, might actually fix it vs masking (not that it would be bad to keep the <=) | 18:32 |
| morganfainberg | ayoung, also you have an extra "e" after report | 18:33 |
| morganfainberg | assertReporteEvent... | 18:33 |
| ayoung | and it is camel case | 18:33 |
| morganfainberg | ayoung, hehe | 18:34 |
| morganfainberg | ayoung, the camel case is consistent with other assert* methods | 18:34 |
| ayoung | morganfainberg, I don't have an extra e. I am missing a d | 18:34 |
| morganfainberg | ayoung, just noticed the extra e :P | 18:34 |
| morganfainberg | ahhhh | 18:34 |
| morganfainberg | ayoung, are we really getting tests with microsecond skew? | 18:35 |
| ayoung | morganfainberg, no idea | 18:35 |
| ayoung | hence my reluctance to say "this is the problem" | 18:36 |
| morganfainberg | ayoung, because afaict short of clock drift mid-execution we shouldn't be seeing this error | 18:36 |
| ayoung | but unless the clocks are out of skew, this is the only thing I can think of | 18:36 |
| morganfainberg | ayoung, of the test. | 18:36 |
| ayoung | morganfainberg, I can see issued_at == before_time being true | 18:36 |
| ayoung | should only be on a single CPU | 18:37 |
| morganfainberg | ayoung, correct. and on a single cpu we shouldn't see a drift back? | 18:37 |
| ayoung | not back..just not adnvacing of the clock | 18:37 |
| morganfainberg | ayoung, ah right | 18:38 |
| ayoung | morganfainberg, thing is, I am comfortable making this change. Anything more significant would indicate a larger issue. I'd like to see if this fixes it | 18:38 |
| ayoung | but let me report the error, too | 18:38 |
| *** raildo has quit IRC | 18:41 | |
| *** raildo has joined #openstack-keystone | 18:41 | |
| morganfainberg | ayoung, lets call this closed | 18:45 |
| morganfainberg | ayoung, if it hasn't solved it, we should reopen | 18:45 |
| ayoung | morganfainberg, sounds good | 18:45 |
| morganfainberg | ayoung, i'm comfortable with this being a test bug | 18:46 |
| morganfainberg | ayoung, i'm going to defer the DI fix until J, we'll supersede this bug w/ the real fix/bug/bp. this only affects tests and tbh, is going to be a way worse fix than leaving it as is. | 18:47 |
| morganfainberg | dolphm, ^ | 18:47 |
| *** amcrn has joined #openstack-keystone | 18:51 | |
| *** gokrokve has joined #openstack-keystone | 18:54 | |
| bknudson | I think I tried to add some code to the DI once where it would reject any attempt to re-assign a provider... gave up because of the reload backends multiple times. | 18:58 |
| morganfainberg | bknudson, yeah | 19:00 |
| morganfainberg | bknudson, i think we need to just make it a Juno target and really fix things up | 19:01 |
| morganfainberg | bknudson, we can avoid just tacking code onto it in that case | 19:01 |
| openstackgerrit | ayoung proposed a change to openstack/keystone: Comparisons should account for instantaneous test execution https://review.openstack.org/81879 | 19:03 |
| dolphm | morganfainberg: ack | 19:04 |
| ayoung | morganfainberg, ^^ should better report a failure if it happens in the future | 19:06 |
| morganfainberg | ayoung, saw it, looks way better | 19:06 |
| ayoung | dolphm, 7c9746c4 should have added in that it fixed the other RC blocker bug | 19:22 |
| dolphm | ayoung: which bug? | 19:22 |
| ayoung | REVOCATION_CACHE_EXPIRATION_TIME | 19:22 |
| ayoung | https://bugs.launchpad.net/keystone/+bug/1291423 dolphm | 19:23 |
| uvirtbot | Launchpad bug 1291423 in keystone "revocation events sync slows responses to all authenticated calls" [High,Triaged] | 19:23 |
| dstanek | ayoung: are there tests that test taking an LDAP response and converting to a model object? | 19:24 |
| ayoung | dolphm, there was a dependency issue that motivated me to make it use the same caching as the rest of dogpile | 19:24 |
| dolphm | ayoung: oh yeah! | 19:25 |
| ayoung | er...the same dogpile approach to caching? | 19:25 |
| ayoung | I'll update the bug | 19:25 |
| *** daneyon has quit IRC | 19:25 | |
| dolphm | ayoung: oh, i just did | 19:26 |
| ayoung | dolphm, I knew I had made that change, and it was driving me crazy that I couldn't find it. Finally, I said,\ "ah must have deleted that branch (even though I don't see it in git reflog must have not checked it in" and went to rewrite it....and I noticed it was already implemented | 19:27 |
| * ayoung scares /me | 19:27 | |
| dolphm | ayoung: i thought i had seen a patch as well, but figured it must not have been in gerrit | 19:27 |
| openstackgerrit | David Stanek proposed a change to openstack/keystone: expires_at should be in a tuple not turned into one https://review.openstack.org/81894 | 19:28 |
| ayoung | dolphm, is stevedor something to be embraced or resisted? | 19:29 |
| dolphm | ayoung: -1 due to risk of market confusion with stevemar | 19:29 |
| ayoung | Heh | 19:29 |
| morganfainberg | dolphm, ++ | 19:30 |
| morganfainberg | dolphm, https://bugs.launchpad.net/keystone/+bug/1294862 seems to be unrelated to memcache specifically | 19:30 |
| uvirtbot | Launchpad bug 1294862 in keystone "Token expiration time with memcache->kvs->dogpile is wrong" [Medium,New] | 19:30 |
| morganfainberg | dolphm, i'm setting up a UTC+1 devstack to poke at it | 19:30 |
| dstanek | ayoung: ^ i have no idea where to add a test for that, but i'm pretty sure that there is some issue with Trust.expires_at as it related to LDAP | 19:30 |
| morganfainberg | dolphm, but reread the bug report, this might be an auth_token bug | 19:31 |
| *** andreaf has quit IRC | 19:31 | |
| dolphm | morganfainberg: auth_token comparing against the wrong TZ? | 19:31 |
| ayoung | morganfainberg, looks like a config error | 19:31 |
| ayoung | UTC + conf.token.expiration, == now if conf.token.expiration == 0 | 19:32 |
| dolphm | morganfainberg: given our 1 hour tokens, we shall detect time zone issues with great precision | 19:32 |
| morganfainberg | dolphm, yeah | 19:32 |
| morganfainberg | ayoung, expiration = 3600 | 19:32 |
| dstanek | ayoung: i think he means the the machine timezone is UTC+1 | 19:32 |
| morganfainberg | ayoung, and the system time is UTC+1 | 19:33 |
| morganfainberg | ayoung, i talked w/ him on irc about it | 19:33 |
| morganfainberg | dolphm, i like 1h tokens! | 19:33 |
| dstanek | it's likely that auth_token just isn't using UTC | 19:33 |
| dolphm | which means tokens might be immediately expired if it's comparing against system time without regard for timezones | 19:33 |
| ayoung | so token is issued withlocal time, but expires at is set with UTC? | 19:33 |
| morganfainberg | ayoung, it should all be converted to UTC | 19:33 |
| dolphm | *should* | 19:33 |
| morganfainberg | ayoung, but he claims keystone API calls worked, just not nova | 19:34 |
| morganfainberg | ayoung etc | 19:34 |
| morganfainberg | ayoung so i think it might actually be an auth_token issue | 19:34 |
| dolphm | morganfainberg: ++ | 19:34 |
| morganfainberg | dolphm, it might be the same memcache bug we had w/ keystone actually | 19:34 |
| ayoung | or memcache | 19:34 |
| ayoung | but if it were memcache, token would be valid first time and invalid on second | 19:34 |
| morganfainberg | ayoung, yeah so i'm setting up a devstack to get more info | 19:35 |
| morganfainberg | ayoung, since he's UTC+1 (i think) hard to sync up sometimes | 19:35 |
| ayoung | morganfainberg, in which time zone? | 19:35 |
| morganfainberg | ayoung, oslo | 19:35 |
| ayoung | you can set the machine tz to be oslo time then | 19:35 |
| morganfainberg | ayoung, that is what i did | 19:35 |
| ayoung | I know...you are faster than I am on the uptake. We all know that | 19:36 |
| morganfainberg | ayoung, just need to stack up to run checks | 19:36 |
| ayoung | https://github.com/openstack/python-keystoneclient/blob/master/keystoneclient/middleware/auth_token.py#L312 | 19:36 |
| ayoung | https://github.com/openstack/python-keystoneclient/blob/master/keystoneclient/middleware/auth_token.py#L339 | 19:36 |
| morganfainberg | ayoung, oh god, we have will expire soon and confirm_not_expired | 19:36 |
| morganfainberg | the parse should be fine in confirm | 19:37 |
| morganfainberg | that is related to expires_at which we make sure is normalized inside keystone | 19:37 |
| ayoung | def get_admin_token(self): | 19:37 |
| ayoung | https://github.com/openstack/python-keystoneclient/blob/master/keystoneclient/middleware/auth_token.py#L680 | 19:37 |
| morganfainberg | ayoung, yeah will expire soon is prob it. | 19:37 |
| morganfainberg | the admin token is always expired | 19:37 |
| ayoung | why do we need to normalize? Isn;t that suspect? | 19:37 |
| ayoung | nah, its like in 30 seconds...so go get a new one | 19:38 |
| ayoung | if that is a problem, so is the other | 19:38 |
| dolphm | ayoung: i don't know what normalize does offhand, but we call it consistently every time we do parse_isotime() | 19:38 |
| morganfainberg | dolphm, it makes sure a datetime object is UTC tz | 19:38 |
| ayoung | https://github.com/openstack/python-keystoneclient/blob/master/keystoneclient/openstack/common/timeutils.py#L68 | 19:38 |
| ayoung | drops the time zone | 19:38 |
| dolphm | morganfainberg: and if it's naive? | 19:39 |
| morganfainberg | dolphm, parse_isotime doesn't enforce TZ-aware objects | 19:39 |
| ayoung | other way 'round, I think | 19:39 |
| morganfainberg | ayoung, ++ | 19:39 |
| morganfainberg | ayoung, yeah sorry inverted it | 19:39 |
| ayoung | so...normalize is suspect. What are we using non TZ times for? | 19:40 |
| ayoung | utcnow is non TZ, isn't it? | 19:40 |
| morganfainberg | ayoung, because otherwise we need to do TZ math | 19:40 |
| morganfainberg | ayoung, we want everything UTC | 19:40 |
| morganfainberg | ayoung, so we say if it has a TZ make it UTC and unset the offset | 19:40 |
| ayoung | but that just drops the TZ | 19:40 |
| morganfainberg | ayoung, it does the offset math as well | 19:41 |
| morganfainberg | return timestamp.replace(tzinfo=None) - offset | 19:41 |
| morganfainberg | so it undoes the offset and drops the TZ | 19:41 |
| ayoung | I'd almost say "convert it to UTC with the Z on it" | 19:42 |
| morganfainberg | ayoung, six of one. | 19:43 |
| ayoung | instead of implicit "oh, naive? mst be UTC" | 19:43 |
| ayoung | nah, explicit instead of implicit | 19:43 |
| morganfainberg | ayoung, iirc the only place we get datetiem objects is utcnow | 19:43 |
| dolphm | morganfainberg: until we make a mistake | 19:44 |
| morganfainberg | but isotime is tz aware | 19:44 |
| morganfainberg | dolphm, we could propose extra features to timeutils and make everything TZ UTC datetime objects instead | 19:44 |
| morganfainberg | i don't think we should do it internal to keystone alone though | 19:45 |
| dolphm | ayoung: i can only imagine that something really doesn't like TZ-aware datetime objects, so that became the default? | 19:46 |
| dolphm | ayoung: but i agree | 19:46 |
| morganfainberg | dolphm, either that or everything was naive first | 19:47 |
| morganfainberg | dolphm, and it was easier to convert the few originally non-naive objects | 19:47 |
| *** jamielennox is now known as jamielennox|away | 19:48 | |
| *** zhiyan_ is now known as zhiyan | 19:50 | |
| *** raildo has quit IRC | 19:52 | |
| *** raildo has joined #openstack-keystone | 19:52 | |
| dolphm | bknudson: did you see this? https://bugs.launchpad.net/grenade/+bug/1294971 | 19:54 |
| uvirtbot | Launchpad bug 1294971 in keystone "gate-grenade-dsvm-partial-ncpu Failure in upgrade-keystone" [Critical,Triaged] | 19:54 |
| dolphm | bknudson: looks like that started (mostly in stable/havana) in the last day or two, if i'm reading the hits correctly | 19:55 |
| bknudson | dolphm: I did see that... a change went in recently to switch to use _ | 19:55 |
| bknudson | dolphm: https://review.openstack.org/#/c/58766/ | 19:55 |
| bknudson | dolphm: so one theory -- there's another problem that's occurring, raising an exception | 19:56 |
| bknudson | and keystone isn't handling the case where an exception is thrown properly anymore. | 19:56 |
| bknudson | as in, an exception gets thrown before the _ is installed | 19:56 |
| dolphm | bknudson: we still have a couple gettextutils.enable_lazy()'s | 19:56 |
| bknudson | dolphm: right, you still have to do that. | 19:57 |
| bknudson | dolphm: but you don't do gettext.install() anymore. | 19:57 |
| dolphm | bknudson: ah, that's what i'm thinking of | 19:57 |
| bknudson | it used to be gettextutils.install() that added _ to builtins | 19:57 |
| bknudson | now every module has to import _ to get it | 19:57 |
| dolphm | bknudson: ah, keystone-all uses _() but doesn't import it itself | 20:00 |
| dstanek | dolphm: ugg..pep8 probably doesn't look at non .py Python files | 20:01 |
| bknudson | dolphm: when there's a socket error. | 20:01 |
| dolphm | dstanek: yep! bknudson: yep! | 20:01 |
| bknudson | maybe this is how keystone fails when someone's using its port | 20:01 |
| dolphm | bknudson: lol exactly | 20:01 |
| openstackgerrit | Dolph Mathews proposed a change to openstack/keystone: explicitly import gettext function https://review.openstack.org/81903 | 20:03 |
| dolphm | dstanek: bknudson: https://review.openstack.org/#/c/81903/ | 20:04 |
| *** raildo has quit IRC | 20:04 | |
| bknudson | 2014-03-20 15:06:26.231 CRITICAL keystone [-] NameError: global name '_' is not defined | 20:06 |
| bknudson | that's what it said when keystone was already running | 20:06 |
| bknudson | now it does 2014-03-20 15:06:15.157 CRITICAL keystone [-] error: [Errno 98] Address already in use | 20:06 |
| *** daneyon has joined #openstack-keystone | 20:08 | |
| ayoung | dolphm, are we following suit? http://lists.openstack.org/pipermail/openstack-dev/2014-March/030576.html | 20:09 |
| dolphm | ayoung: i'd love to!@ | 20:09 |
| lbragstad | ++, that would be nice | 20:09 |
| bknudson | ayoung: dolphm: no objection from me | 20:09 |
| ayoung | dolphm, then lets request http://git.openstack.org/cgit/openstack/keystone-specs | 20:09 |
| dolphm | ayoung: i'll put it on the agenda for next week | 20:09 |
| bknudson | seconded | 20:09 |
| ayoung | Thirded | 20:09 |
| dolphm | fourthded | 20:10 |
| morganfainberg | i kinda think gerrit is the wrong tool for the job :( | 20:11 |
| * morganfainberg wants storyboard | 20:11 | |
| morganfainberg | or a tool that is more in line with this type of workflow | 20:12 |
| morganfainberg | i don't think it's a bad idea though to have this | 20:12 |
| morganfainberg | just saying git / gerrit feels like the wrong tool for the job :P | 20:12 |
| morganfainberg | (no i don't have an alternative) | 20:12 |
| bknudson | I believe nova's goal was to get better quality blueprints... not just "do this". | 20:13 |
| morganfainberg | bknudson, and thats fine, i still think git is a really atrocious tool for that | 20:13 |
| bknudson | they want "do this this way" | 20:13 |
| lbragstad | using gerrit gets a lot of eyes on a lot of different blueprints | 20:13 |
| morganfainberg | lbragstad, i firmly believe that this is solving an issue w/ Launchpad with something better but also not great. | 20:14 |
| morganfainberg | and since i don't have an alternative.... | 20:14 |
| morganfainberg | no reason not to use better tools than we have | 20:14 |
| dolphm | morganfainberg: i want to hear ttx's feedback, because he's going to be impacted first | 20:15 |
| morganfainberg | dolphm, sounds good to me. | 20:15 |
| dolphm | morganfainberg: all his LP tooling will be shot, and i don't know what he's doing that can/can't be adapted to this | 20:15 |
| dolphm | morganfainberg: things like milestone targets are relevant | 20:15 |
| morganfainberg | dolphm, i think this new "system" will rapidly go away if we get storyboard off the ground | 20:16 |
| morganfainberg | since if we need added features/functionality we can then add it | 20:16 |
| dolphm | morganfainberg: what IS storyboard?! | 20:16 |
| lbragstad | isn't that ttx's project? | 20:16 |
| dolphm | morganfainberg: i've heard about it in passing a few times, and even read the meeting log today... | 20:16 |
| morganfainberg | dolphm, meant to be a replacement for LP because LP has almost no support | 20:16 |
| dolphm | lbragstad: he's definitely involved | 20:16 |
| lbragstad | ahh gotcha | 20:16 |
| morganfainberg | dolphm, LP basically has no forward momentum (afaict there are like 2 guys doing bare minimum maintenance), it's got a ton of limiations, and isn't a good solution | 20:17 |
| dolphm | morganfainberg: reading russell's email again, i don't think he wants to supersede LP, just supplement it? | 20:17 |
| morganfainberg | dolphm, correct | 20:17 |
| morganfainberg | dolphm, and if we have a new non-sucky system instead of LP i think this supplement will be redundant | 20:18 |
| morganfainberg | LP's whiteboards are useless, searching is bad, bps aren't good. this supplements that and aims to solve some of those issues. | 20:18 |
| morganfainberg | storyboard is afaict meant to replace LP with something we can manage/maintain/enhance which would make this supplement redundant | 20:19 |
| morganfainberg | i am sure git is the wrong tool for this, but it is far superior to LP alone | 20:20 |
| morganfainberg | so, i don't thing we shouldn't follow in suit. | 20:20 |
| morganfainberg | erm, i think we should (no double negatives) | 20:20 |
| morganfainberg | if ttx doesn't say this is all around terrible | 20:20 |
| dolphm | morganfainberg: i take back most of my concern for any objection from ttx | 20:23 |
| dolphm | morganfainberg: this is really just applying our /identity-api/ philosophy to everything blueprinty | 20:23 |
| morganfainberg | dolphm, yep | 20:24 |
| dolphm | morganfainberg: and he couldn't care less about /identity-api/ i imagine | 20:24 |
| dolphm | morganfainberg: replied on list -- hopefully we can simplify it | 20:24 |
| morganfainberg | dolphm, now if LP didn't suck, i don't think this would be needed | 20:24 |
| *** flaper87|afk is now known as flaper87 | 20:25 | |
| morganfainberg | dolphm, ++ on you comments :) | 20:28 |
| *** dstanek_afk has joined #openstack-keystone | 20:30 | |
| *** dstanek has quit IRC | 20:32 | |
| *** pcargnel has joined #openstack-keystone | 20:32 | |
| *** rwsu has quit IRC | 20:37 | |
| *** marekd|bbl is now known as marekd | 20:38 | |
| *** daneyon has quit IRC | 20:40 | |
| *** dims has quit IRC | 20:46 | |
| pcargnel | Hi! I'll appreciate if someone could take a look on this one: https://review.openstack.org/#/c/80368/7. Thanks :) | 20:52 |
| dstanek_afk | pcargnel: why rename to handle_*? | 20:53 |
| *** dstanek_afk is now known as dstanek | 20:54 | |
| *** jamielennox|away is now known as jamielennox | 20:55 | |
| *** pcargnel has quit IRC | 20:56 | |
| *** flaper87 is now known as flaper87|afk | 20:56 | |
| dstanek | i'm also not sold on event_callbacks in Manager instances. what happens if multiple manager are instantiated? | 20:57 |
| dstanek | morganfainberg: ^ notification/singleton question | 20:57 |
| ayoung | dstanek, depends. Are ou talking consuming or producing events. Either way, So long as the event is produced/consumed once or in a idempotent way, it is ok | 21:01 |
| dstanek | ayoung: consuming events | 21:01 |
| *** dims has joined #openstack-keystone | 21:02 | |
| dstanek | ayoung: does dependency make sure only one handler per class is registered? | 21:02 |
| morganfainberg | dstanek, the first one ... i think is registered | 21:02 |
| morganfainberg | dstanek, and the subsequent instantiations are not | 21:03 |
| dstanek | my connectivity is really bad here...so i may drop on and off | 21:03 |
| dstanek | morganfainberg: ok, i had not seen code to do that | 21:03 |
| morganfainberg | dstanek, let me check but i think the _register_callbacks does htat | 21:04 |
| *** rwsu has joined #openstack-keystone | 21:04 | |
| dstanek | morganfainberg: no, it uses a set and that won't work for bound methods | 21:04 |
| dstanek | unless i'm misremembering | 21:04 |
| morganfainberg | dstanek, it works for bound methods since it does registration post instantiation | 21:05 |
| dstanek | morganfainberg: isn't that why it wouldn't work? | 21:06 |
| morganfainberg | dstanek, actually os it looks like if you instantiate a class mutiple times you'll have multiuple callbacks registered | 21:06 |
| * morganfainberg check 100% | 21:06 | |
| dstanek | each bound method hashes differently because it is bound to a different instance | 21:07 |
| *** jaypipes has quit IRC | 21:07 | |
| morganfainberg | dstanek, the original code only allowed for a single callback | 21:07 |
| morganfainberg | dstanek, iirc but that is not how this works. | 21:08 |
| morganfainberg | dstanek, yeah each instantiation of the manager listens for events | 21:08 |
| morganfainberg | dstanek, i think we need to move to a model where a manager is instantiated exactly 1 time. | 21:09 |
| ayoung | So long as we are only listening to internal events, we should be OK. | 21:09 |
| ayoung | assume a 4 worker scenario | 21:09 |
| morganfainberg | ayoung, yeah we're only listening for internal callbacks | 21:10 |
| ayoung | if there were crosstalk between them, there would be a problem, but for internal only we should be OK | 21:10 |
| dstanek | but if two callback try to delete the same user we'll produce a 500 | 21:10 |
| morganfainberg | ayoung, it's just extra overhead to call the event if it will be a noop | 21:10 |
| ayoung | and "internal" does not mean to avoid routing them to the outside world | 21:10 |
| morganfainberg | dstanek, if the callback is smart, it shouldn't try if the user doesn't exist, right? | 21:11 |
| dstanek | morganfainberg's patch gets us much better off my not creating instances or managers, but nothing prevents it right now | 21:11 |
| ayoung | dstanek, we don't perform operations like that on callbacks, | 21:11 |
| ayoung | just create revocation events for now, or do we? | 21:11 |
| morganfainberg | ayoung, no but pcargnel's patch does introduce that | 21:11 |
| ayoung | link? | 21:11 |
| morganfainberg | ayoung, https://review.openstack.org/#/c/80368/7 | 21:12 |
| morganfainberg | if we are mucking with all that stuff moving to events would be better. | 21:12 |
| morganfainberg | but i'm seeing some oddities that make that hard | 21:12 |
| dstanek | morganfainberg: the callback could catch the SQL exception | 21:12 |
| ayoung | something really wrong there | 21:12 |
| ayoung | those operations should not be on the assignment API any longer | 21:13 |
| jamielennox | dolphm: https://review.openstack.org/#/c/60752/ - can you please have another look | 21:13 |
| morganfainberg | dstanek, also https://review.openstack.org/#/c/80368/7/keystone/contrib/revoke/core.py trying to figure out line 108 | 21:13 |
| morganfainberg | dstanek, how is that not raising an exception? | 21:13 |
| ayoung | LDAP is a little wonky in the the assignment API assumes an LDAP identity, but you still should not be calling delete_user on assignmnet | 21:13 |
| ayoung | that should only be in identity | 21:13 |
| morganfainberg | indexed lookup on a string? | 21:14 |
| morganfainberg | ayoung, if the assignment manager needs to take an action on user being deleted, where do you put that? | 21:14 |
| ayoung | LDAP Identity Driver does not call delete_user or delete_group on the LDAP assignment api | 21:14 |
| ayoung | OK, that makes more sense | 21:14 |
| ayoung | I had it backwards | 21:14 |
| dstanek | morganfainberg: what string? | 21:14 |
| morganfainberg | dstanek, the payload['resource_info'] | 21:14 |
| morganfainberg | dstanek, that is just the iud | 21:14 |
| morganfainberg | id* | 21:15 |
| morganfainberg | maybe we're missing tests for the places that occurs in this patchset | 21:15 |
| ayoung | morganfainberg, I'm almost willing to say that one should be solved by "LDAP to LDAP secret backchannel negotiations" | 21:15 |
| nkinder | morganfainberg: when you have a chance, do you mind re-reviewing this one for me? | 21:15 |
| dstanek | morganfainberg: ah, so it is | 21:15 |
| nkinder | https://review.openstack.org/#/c/73935/ | 21:15 |
| morganfainberg | nkinder, sure thing | 21:15 |
| ayoung | But the question is if we want to handle these types of events in the general case... | 21:15 |
| morganfainberg | ayoung, sure, but ok so identity LDAP (r/w) w/ sql assignment, user is deleted | 21:16 |
| morganfainberg | does that make an explicit assingment call? or just issue a callback event? | 21:16 |
| * morganfainberg swears he can type assignment | 21:16 | |
| morganfainberg | ayoung, i would like to think that we can use the callbacks for most things inter-manager that need to happen because of X | 21:18 |
| morganfainberg | ayoung, it's not that often that stuff needs to happen | 21:19 |
| morganfainberg | ayoung, but i'm not 100% sold. | 21:20 |
| *** nkinder has quit IRC | 21:20 | |
| morganfainberg | ayoung, i prefer a "just listen for X and act if it occurs" rather than needing to change code in X Y and Z to support a manager needing to cleanup post user_delete | 21:20 |
| morganfainberg | dstanek, this is along the same lines of using hooks vs explicit changes for deps. | 21:21 |
| *** jamielennox is now known as jamielennox|away | 21:22 | |
| dstanek | morganfainberg: in my mind a callback would be an instance with dependencies injected; indexed in Notifications by the class | 21:22 |
| morganfainberg | dstanek, so (object) with *_api and a method that knows what to do w/ an event? | 21:23 |
| dstanek | morganfainberg: this is similar to what i have done in the past | 21:24 |
| dstanek | i like functions as call backs, but i needed to have the injector to it's magic | 21:25 |
| dstanek | in this case register would create the instance | 21:25 |
| *** flaper87|afk is now known as flaper87 | 21:27 | |
| morganfainberg | dstanek, so @dep.provider registers the callback? | 21:27 |
| morganfainberg | dstanek, still? | 21:27 |
| dstanek | i need to setup and irc bouncer | 21:27 |
| morganfainberg | dstanek, ++ they're useful | 21:28 |
| dstanek | morganfainberg: no, maybe another decorator like you do in celery | 21:28 |
| morganfainberg | dstanek, ah ok | 21:28 |
| morganfainberg | dstanek, sure. not opposed to that. | 21:28 |
| dstanek | morganfainberg: part of my blinker demo should be to show some of these ideas | 21:29 |
| morganfainberg | dstanek, here is another that i'm not sure how it passes tests https://review.openstack.org/#/c/80368/7/keystone/credential/core.py | 21:30 |
| morganfainberg | line 115 | 21:30 |
| morganfainberg | dstanek, or even https://review.openstack.org/#/c/80368/7/keystone/credential/backends/sql.py | 21:31 |
| dstanek | the tests can't be testing this | 21:31 |
| morganfainberg | dstanek, obviously | 21:31 |
| morganfainberg | oh gah | 21:33 |
| morganfainberg | dstanek, https://review.openstack.org/#/c/80368/7/keystone/identity/backends/sql.py line 238 | 21:33 |
| *** daneyon has joined #openstack-keystone | 21:33 | |
| morganfainberg | i don't know how this passed tests. | 21:34 |
| morganfainberg | i'm looking at code that this should have failed on. | 21:34 |
| *** flaper87 is now known as flaper87|afk | 21:34 | |
| *** david-lyle has quit IRC | 21:37 | |
| openstackgerrit | Andreas Jaeger proposed a change to openstack/keystone: Remove obsolete po entries - they break translation jobs https://review.openstack.org/81925 | 21:37 |
| morganfainberg | dstanek, i... i am scared by this test working. | 21:44 |
| *** dstanek has quit IRC | 21:46 | |
| *** gokrokve has quit IRC | 21:52 | |
| *** gokrokve has joined #openstack-keystone | 21:52 | |
| *** zhiyan is now known as zhiyan_ | 21:55 | |
| *** gokrokve has quit IRC | 21:56 | |
| *** marcoemorais has quit IRC | 21:59 | |
| *** marcoemorais has joined #openstack-keystone | 21:59 | |
| *** marekd is now known as marekd|away | 22:10 | |
| *** nkinder has joined #openstack-keystone | 22:10 | |
| *** marcoemorais has quit IRC | 22:20 | |
| *** gokrokve has joined #openstack-keystone | 22:23 | |
| *** marcoemorais has joined #openstack-keystone | 22:24 | |
| *** marcoemorais has quit IRC | 22:33 | |
| *** marcoemorais has joined #openstack-keystone | 22:33 | |
| *** david-lyle has joined #openstack-keystone | 22:38 | |
| *** flaper87|afk is now known as flaper87 | 22:50 | |
| *** stevemar has quit IRC | 22:52 | |
| *** gokrokve has quit IRC | 23:00 | |
| *** gokrokve has joined #openstack-keystone | 23:01 | |
| *** daneyon has quit IRC | 23:05 | |
| *** gokrokve has quit IRC | 23:05 | |
| *** daneyon has joined #openstack-keystone | 23:05 | |
| *** flaper87 is now known as flaper87|afk | 23:07 | |
| *** jamielennox|away is now known as jamielennox | 23:08 | |
| *** richm has quit IRC | 23:13 | |
| *** bknudson has quit IRC | 23:16 | |
| *** henrynash has quit IRC | 23:16 | |
| *** thedodd has quit IRC | 23:17 | |
| *** daneyon has quit IRC | 23:23 | |
| *** david-lyle has quit IRC | 23:31 | |
| *** dstanek has joined #openstack-keystone | 23:46 | |
| jamielennox | where are with with RC freeze? | 23:51 |
| jamielennox | i only just saw the -1 on https://review.openstack.org/#/c/78068/6 | 23:51 |
| jamielennox | but i'm wondering if i'm too late to fix it now | 23:51 |
| *** henrynash has joined #openstack-keystone | 23:51 | |
| *** browne has quit IRC | 23:54 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!