| openstackgerrit | Jamie Lennox proposed a change to openstack/python-keystoneclient: Handle URLs via the session and auth_plugins https://review.openstack.org/60752 | 00:01 |
|---|---|---|
| *** gokrokve has quit IRC | 00:04 | |
| openstackgerrit | Jamie Lennox proposed a change to openstack/keystone: Change the default version discovery URLs https://review.openstack.org/78068 | 00:24 |
| *** nkinder_ has joined #openstack-keystone | 00:24 | |
| openstackgerrit | Jamie Lennox proposed a change to openstack/keystone: Change the default version discovery URLs https://review.openstack.org/78068 | 00:32 |
| *** stevemar has quit IRC | 00:32 | |
| *** gokrokve has joined #openstack-keystone | 00:33 | |
| *** richm has quit IRC | 00:41 | |
| *** browne has quit IRC | 00:41 | |
| *** petertoft has joined #openstack-keystone | 00:46 | |
| *** mhu has quit IRC | 00:49 | |
| *** petertoft has quit IRC | 00:50 | |
| *** browne has joined #openstack-keystone | 00:52 | |
| *** zhiyan_ has quit IRC | 00:57 | |
| *** jraim has quit IRC | 00:57 | |
| *** zigo has quit IRC | 00:58 | |
| *** tellesnobrega has quit IRC | 00:58 | |
| *** huats has quit IRC | 00:58 | |
| *** mhu has joined #openstack-keystone | 00:58 | |
| openstackgerrit | A change was merged to openstack/keystone: Configurable temporary directory for tests https://review.openstack.org/79217 | 00:58 |
| *** zhiyan` has joined #openstack-keystone | 00:58 | |
| *** jordant has quit IRC | 00:58 | |
| *** huats has joined #openstack-keystone | 00:58 | |
| *** huats has quit IRC | 00:58 | |
| *** huats has joined #openstack-keystone | 00:58 | |
| *** jraim has joined #openstack-keystone | 00:58 | |
| *** openstack has joined #openstack-keystone | 01:04 | |
| *** marcoemorais has quit IRC | 01:10 | |
| *** devlaps has quit IRC | 01:10 | |
| *** gokrokve has quit IRC | 01:17 | |
| *** mberlin1 has joined #openstack-keystone | 01:19 | |
| *** henrynash has quit IRC | 01:23 | |
| *** gokrokve has joined #openstack-keystone | 01:25 | |
| jamielennox | morganfainberg, lbragstad, gyee, ayoung whoever else is here: can you have a look at https://review.openstack.org/#/c/77026/ and at least let me know if the approach is good | 01:25 |
| ayoung | jamielennox, I already acked that | 01:25 |
| jamielennox | ayoung: ah indeed | 01:26 |
| jamielennox | is the gate particularly stuffed at the moment? | 01:26 |
| ayoung | jamielennox, what about dstanek_afk 's comment? | 01:26 |
| *** zigo has quit IRC | 01:27 | |
| jamielennox | yea, i got that | 01:27 |
| jamielennox | it does improve things | 01:27 |
| *** zigo has joined #openstack-keystone | 01:33 | |
| *** dstanek_afk is now known as dstanek | 01:34 | |
| dstanek | hey all | 01:34 |
| dstanek | jamielennox: i really like that positional decorator | 01:35 |
| jamielennox | dstanek: excellent | 01:35 |
| dstanek | i'll go through it for a real review in a few | 01:35 |
| jamielennox | me too - i really want to use it and i don't know why it's been sitting so long | 01:35 |
| *** gokrokve has quit IRC | 01:38 | |
| dstanek | jamielennox: do you have a few for some realtime questions? instead of me asking in the review? | 01:41 |
| jamielennox | sure | 01:41 |
| dstanek | what do you plan on actually using it for? | 01:42 |
| jamielennox | dstanek: there is a follow up patch where i start to use it | 01:42 |
| jamielennox | the point though is to seperate optional positional arguments and keyword arguments | 01:43 |
| jamielennox | for example with Session() i wanted the first parameter to be the auth plugin | 01:43 |
| jamielennox | however in at least the first review there was no auth plugins yet | 01:43 |
| jamielennox | so i gave it a bunch of kwargs, but now when i want to put in a positional auth argument it isn't backwards compatible | 01:44 |
| *** gokrokve has joined #openstack-keystone | 01:44 | |
| dstanek | jamielennox: is that protecting out code from calling it incorrectly or is it preventing 3rd party? | 01:44 |
| jamielennox | dstanek: it's giving us a way to better define our public interfaces so others don't use them incorrectly | 01:45 |
| jamielennox | dstanek: see also: http://legacy.python.org/dev/peps/pep-3102/ | 01:46 |
| jamielennox | which came in in python3 | 01:46 |
| jamielennox | i'll see if i can find a beter link | 01:46 |
| dstanek | jamielennox: ok, that's what i thought - i just wouldn't want to see that used all over the place | 01:46 |
| jamielennox | yea, it's for external interfaces just so we have some more control over them in future | 01:47 |
| dstanek | ok, cool - the inspect line i gave you actually came from one of the pep8 checks i've been working on adding | 01:48 |
| dstanek | jamielennox: it doesn't look like POSITIONAL_NONE is actually used | 01:49 |
| openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Fixup region description uniqueness https://review.openstack.org/79159 | 01:50 |
| jamielennox | it's not, it's there for completeness if you want to turn it off | 01:50 |
| morganfainberg | lbragstad, posted updated review | 01:50 |
| morganfainberg | i could have done it with a temporary unique constraint on the migrate but i felt that it was too mechanical, the region.<attr> seemed fine for an insert() call instead of using a .add() call | 01:51 |
| dstanek | jamielennox: you would eventually add functionality to turn it off? | 01:55 |
| ayoung | dstanek, Its my fult. I started nacking his patches due to him changing the order or params | 01:55 |
| ayoung | and I was the one that asked him to do it in the first place, too | 01:56 |
| dstanek | :-) | 01:56 |
| morganfainberg | dstanek, when you have a few, ^ the region uniqueness, any issues with that impl (this is something that is needed for I release) | 01:56 |
| dstanek | i'm just trying to understand it - didn't know if there was more to come or if it was something that should have been deleted with something else | 01:56 |
| jamielennox | dstanek: not internally, but it's a library so you can set utils.postitional_enforcecement = utils.POSITIONAL_NONE from your app and disable the warnings | 01:56 |
| morganfainberg | jaypipes, ping ^ new spin on region unique description fix | 01:56 |
| jamielennox | if you can flip between a warning and an error i figured you should be able to disable it as well | 01:57 |
| dstanek | morganfainberg: sure i'll take a look in a sec | 01:57 |
| jamielennox | and if you disable it then it's your own fault when something breaks later | 01:57 |
| morganfainberg | dstanek, yeah np, figured you would once the current convo was done | 01:57 |
| jaypipes | morganfainberg: +1 from me. | 01:58 |
| *** gokrokve has quit IRC | 01:58 | |
| jamielennox | i use that method because in testing i set utils.postitional_enforcecement = utils.POSITIONAL_EXCEPT so if you violate the rules in testing it fails | 01:58 |
| jamielennox | but defaults to a warning | 01:58 |
| morganfainberg | jaypipes, figured that since I hadn't shipped we could do an idempotent change for dropping the index and save the headache of making a table, make a temp table, migrate, drop, rename if we were just creating the region tables cleanly in the first place | 01:59 |
| jaypipes | yup. | 01:59 |
| morganfainberg | i have learned a good deal more about the ORM while I was at it | 01:59 |
| *** gokrokve has joined #openstack-keystone | 02:01 | |
| dstanek | jamielennox: that's actually an interesting feature - may be worth adding a line or two in the docstring | 02:01 |
| openstackgerrit | A change was merged to openstack/keystone: Add missing documentation for enabling oauth1 auth plugin https://review.openstack.org/79708 | 02:07 |
| *** mberlin has joined #openstack-keystone | 02:07 | |
| openstackgerrit | wanghong proposed a change to openstack/keystone: support conventional domain name with one or more dot https://review.openstack.org/79829 | 02:07 |
| *** mberlin1 has quit IRC | 02:08 | |
| *** gokrokve has quit IRC | 02:14 | |
| *** gokrokve has joined #openstack-keystone | 02:15 | |
| *** gokrokve has quit IRC | 02:19 | |
| *** zigo has quit IRC | 02:19 | |
| gyee | jamielennox, so we are using decorators to overcome the shortcomings of Python language? :) | 02:19 |
| jamielennox | gyee: yes | 02:19 |
| gyee | nice | 02:19 |
| jamielennox | i never found a good link, but in python3 you can do | 02:20 |
| jamielennox | def a(arg1, arg2, arg3='x', *, kwarg1='abc') | 02:20 |
| jamielennox | which seperates default positional arguments from keyword only arguments | 02:20 |
| jamielennox | i want that essentially | 02:21 |
| jamielennox | its the runtime enforcement of bknudson's 'if it's a kwarg then use the keyword' | 02:21 |
| gyee | ha | 02:21 |
| gyee | ++ | 02:21 |
| gyee | but then the drawback is that you'll have to specify it everywhere | 02:22 |
| jamielennox | gyee: eh, only really those functions that take a lot of kwargs | 02:23 |
| *** zigo has joined #openstack-keystone | 02:23 | |
| gyee | public interfaces perhaps | 02:24 |
| jamielennox | but i'll do it if i can stop being concerned with backwards compatability for those interfaces | 02:24 |
| jamielennox | (as concerned) | 02:24 |
| morganfainberg | jamielennox, i like that feature | 02:25 |
| gyee | I knew morganfainberg would like it :) | 02:25 |
| gyee | does it work with inheritance? | 02:27 |
| jamielennox | it's a decorator, so it won't protect the child function but it will be protected when it calls super() | 02:28 |
| jamielennox | it can't really do any better than that | 02:28 |
| jamielennox | i don't think you can/should be able to enforce anything on deriving functions | 02:29 |
| gyee | jamielennox, seem like this is something we can incorporate into pep8 | 02:31 |
| jamielennox | kwargs specified as keywords? | 02:31 |
| jamielennox | hmm, | 02:31 |
| gyee | like scan through all the funcs, automatically add the decorator, and print out any warnings | 02:31 |
| jamielennox | nah, you'd need to do ast parsing to figure out what things allow what params | 02:32 |
| jamielennox | oh | 02:32 |
| jamielennox | i don't know how but it could be done | 02:32 |
| gyee | not sure, was just thinking outloud | 02:32 |
| lbragstad | jamielennox: responed to your review | 02:35 |
| jamielennox | lbragstad: thanks | 02:36 |
| jamielennox | lbragstad: kind of | 02:36 |
| jamielennox | (in answer to question) | 02:36 |
| jamielennox | if you do a @classmethod def(*args, **kwargs) the first arg is always going to be cls | 02:37 |
| lbragstad | jamielennox: ok cool, just curious | 02:37 |
| jamielennox | so if you have def a(self, a, b='b') then you still need to account for self as one of the positional args | 02:37 |
| jamielennox | so by default that's @positional(2) - self and 'a' | 02:38 |
| lbragstad | ok, makes sense. | 02:38 |
| lbragstad | morganfainberg: thanks for pushing another version of migration 043. | 02:39 |
| lbragstad | I appreciate it | 02:39 |
| *** topol has joined #openstack-keystone | 02:39 | |
| ayoung | Wow, not having lookup by name SUCKS! | 02:40 |
| ayoung | basically you have to do: test_projects = admin_client.projects.list(name='hugeproject', | 02:42 |
| ayoung | domain='default') | 02:42 |
| ayoung | remove_users(admin_client, test_projects[0]) | 02:42 |
| gyee | jamielennox, anyway, I don't feel strongly either way. I don't feel we necessarily need it as most of us a good at catching that stuff during code review now. But I don't object if others wants it. | 02:42 |
| jamielennox | gyee: it's not about internal to the code | 02:42 |
| jamielennox | gyee: it's so that we can add new positional arguments after a release has happened and not break compatability | 02:42 |
| ayoung | gyee, it is explicitly not for our code | 02:42 |
| ayoung | it is for 3rd party apps that we break if we change things that were not meant to remain frozen | 02:43 |
| gyee | adding keyword args won't break anything, but if we have to add positional args I would need it needs to be a different interface | 02:44 |
| jamielennox | gyee: why? positional args can be optional | 02:44 |
| ayoung | morganfainberg, I am running a script that creates and then destroys a slew of users, and It is slow. ANd I am seeing a lot of kvs locks, I suspect from the revoke backend | 02:44 |
| ayoung | KVS lock acquired for: os-revoke-events acquire /opt/stack/keystone/keystone/common/kvs/core.py:375 | 02:45 |
| morganfainberg | ayoung, likely | 02:45 |
| ayoung | it might prove to be a problem | 02:45 |
| morganfainberg | that was why i was wondering if we could somehow not need to call syncronize on each call. | 02:45 |
| morganfainberg | erm each validate | 02:45 |
| ayoung | morganfainberg, yeah...I would love to not have to do that | 02:45 |
| morganfainberg | e.g. nothing changed, can we store something? | 02:46 |
| ayoung | of course we can | 02:47 |
| morganfainberg | i think that will speed that srtuff up a lot | 02:47 |
| *** harlowja is now known as harlowja_away | 02:48 | |
| ayoung | morganfainberg, if last_fetch > now - some_time_delta return | 02:49 |
| morganfainberg | ayoung, ++ | 02:49 |
| morganfainberg | yeah | 02:49 |
| *** stevemar has joined #openstack-keystone | 02:50 | |
| *** gyee has quit IRC | 02:54 | |
| *** jordant has joined #openstack-keystone | 02:57 | |
| ayoung | morganfainberg, looks a lot better | 02:58 |
| ayoung | still a lot of locking, running once per second | 02:58 |
| *** amcrn has quit IRC | 03:05 | |
| *** david-lyle has joined #openstack-keystone | 03:09 | |
| morganfainberg | ayoung, i wonder if we could be even more aggressive on that | 03:18 |
| ayoung | morganfainberg, probably, 5 seconds would be fine | 03:18 |
| ayoung | maybe even 10 with no really window | 03:18 |
| ayoung | you'll probably see some test fail, though | 03:19 |
| ayoung | create user, create token, delete user...token won't be revoked for 10 seconds | 03:19 |
| *** rwsu has quit IRC | 03:19 | |
| openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Fixup region description uniqueness https://review.openstack.org/79159 | 03:19 |
| morganfainberg | ayoung, hmm... | 03:20 |
| morganfainberg | ayoung, this almost feels like you should know if a new event came in, only actually sync the data if the event timer has changed. | 03:23 |
| morganfainberg | ayoung, so you could avoid locking all together w/o a new event. obviously if you have a lot of events you'd syncronize each time. | 03:24 |
| ayoung | morganfainberg, yeah. I mean, we only need the backing store to record events | 03:25 |
| ayoung | we could use the in-memory store as the cache and only go to the persisted store if the cache is invalidated, assuming a single keystone server | 03:25 |
| morganfainberg | aye. | 03:26 |
| ayoung | but for multiple servers | 03:26 |
| morganfainberg | hm. | 03:26 |
| ayoung | you need to sync, and probably be a little sloppy about it due to time skew | 03:26 |
| ayoung | actually, we should record the time in the database | 03:26 |
| morganfainberg | or we could rip out that locking stuff and sync stuff and figure the right way to toss proper caching in front of it | 03:26 |
| morganfainberg | and then allow the use of memcache etc for that if you are using multiple servers (same way we allow caching on tokens) | 03:27 |
| morganfainberg | not sure if that is a big win or not | 03:28 |
| morganfainberg | i don't think storing the last event timestamp in the db is the right answer | 03:29 |
| ayoung | jamielennox, is it supposed to be admin_client.users.create(name=name, domain=test_domain) or admin_client.users.create(name=name, domain=test_domain.id) | 03:32 |
| ayoung | actually, I know from empirical that it is the former | 03:33 |
| ayoung | but the later should fail, and it doesn't | 03:33 |
| *** stevemar has quit IRC | 03:35 | |
| jamielennox | ayoung: both should be fine | 03:36 |
| ayoung | second one dropped the domain...I think | 03:37 |
| *** wchrisj has joined #openstack-keystone | 03:37 | |
| jamielennox | ayoung: that's what bas.getid() does | 03:37 |
| ayoung | I'll see if I can reproduce | 03:37 |
| ayoung | I've got other problems, too | 03:37 |
| ayoung | oooh nasty little slipup | 03:38 |
| ayoung | if [Revoke] gets commented out in the config file, but you try to set the driver, you accidentally set the token driver | 03:39 |
| ayoung | Ok...gotta crash | 03:42 |
| jamielennox | ayoung: night | 03:43 |
| *** petertoft has joined #openstack-keystone | 03:48 | |
| *** rwsu has joined #openstack-keystone | 03:50 | |
| *** petertoft has quit IRC | 03:53 | |
| *** dstanek has quit IRC | 03:54 | |
| *** zhiyan` is now known as zhiyan | 04:02 | |
| *** dstanek has joined #openstack-keystone | 04:02 | |
| *** gokrokve has joined #openstack-keystone | 04:05 | |
| *** wchrisj has quit IRC | 04:15 | |
| *** stevemar has joined #openstack-keystone | 04:20 | |
| *** petertoft has joined #openstack-keystone | 04:50 | |
| *** petertoft has quit IRC | 04:54 | |
| *** stevemar has quit IRC | 05:18 | |
| *** petertoft has joined #openstack-keystone | 05:51 | |
| *** petertoft has quit IRC | 05:55 | |
| openstackgerrit | Jenkins proposed a change to openstack/keystone: Imported Translations from Transifex https://review.openstack.org/78525 | 06:03 |
| *** zoresvit1 has joined #openstack-keystone | 06:17 | |
| *** topol has quit IRC | 06:32 | |
| *** gokrokve has quit IRC | 06:46 | |
| *** saju_m has joined #openstack-keystone | 06:52 | |
| openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Do not use keystone.conf.sample in tests https://review.openstack.org/79524 | 07:14 |
| openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Removal of test .conf files https://review.openstack.org/79525 | 07:14 |
| openstackgerrit | Morgan Fainberg proposed a change to openstack/keystone: Move test .conf files to keystone/tests/config_files https://review.openstack.org/79526 | 07:14 |
| *** jaosorior has joined #openstack-keystone | 07:17 | |
| *** bknudson has quit IRC | 07:24 | |
| *** bknudson has joined #openstack-keystone | 07:26 | |
| *** gokrokve has joined #openstack-keystone | 07:42 | |
| *** gokrokve has quit IRC | 07:47 | |
| *** zoresvit1 has quit IRC | 07:53 | |
| *** petertoft has joined #openstack-keystone | 07:53 | |
| openstackgerrit | Marcos FermÃn Lobo proposed a change to openstack/keystone: LDAP global roles and group roles assignments https://review.openstack.org/76568 | 07:53 |
| *** petertoft has quit IRC | 07:57 | |
| *** marekd|away is now known as marekd | 08:00 | |
| *** henrynash has joined #openstack-keystone | 08:01 | |
| *** gokrokve has joined #openstack-keystone | 08:13 | |
| *** henrynash has quit IRC | 08:14 | |
| *** morganfainberg is now known as morganfainberg_Z | 08:17 | |
| *** gokrokve has quit IRC | 08:18 | |
| *** amcrn has joined #openstack-keystone | 08:35 | |
| *** saju_m has quit IRC | 08:41 | |
| *** gokrokve has joined #openstack-keystone | 08:43 | |
| *** gokrokve has quit IRC | 08:47 | |
| *** amcrn_ has joined #openstack-keystone | 08:50 | |
| *** amcrn has quit IRC | 08:53 | |
| *** amcrn_ is now known as amcrn | 08:53 | |
| *** petertoft has joined #openstack-keystone | 09:03 | |
| *** zhiyan is now known as zhiyan_ | 09:06 | |
| *** saju_m has joined #openstack-keystone | 09:09 | |
| openstackgerrit | Marek Denis proposed a change to openstack/keystone: Validate groups presence for federated authn https://review.openstack.org/79284 | 09:17 |
| openstackgerrit | sagar pradhan proposed a change to openstack/keystone: Bug 1273273 :if run 'keystone-manage db_sync' without 'sudo', there should be a prompt that tables not created https://review.openstack.org/79889 | 09:22 |
| *** henrynash has joined #openstack-keystone | 09:22 | |
| *** leseb has joined #openstack-keystone | 09:26 | |
| *** amcrn has quit IRC | 09:38 | |
| *** gokrokve has joined #openstack-keystone | 09:43 | |
| *** gokrokve has quit IRC | 09:48 | |
| *** zoresvit has quit IRC | 10:25 | |
| *** andreaf has joined #openstack-keystone | 10:26 | |
| openstackgerrit | henry-nash proposed a change to openstack/keystone: Ensure v3policysample correctly limits domain_admin access https://review.openstack.org/79897 | 10:28 |
| *** jamielennox is now known as jamielennox|away | 10:34 | |
| *** gokrokve has joined #openstack-keystone | 10:43 | |
| *** gokrokve has quit IRC | 10:48 | |
| *** henrynash has quit IRC | 10:48 | |
| *** saju_m has quit IRC | 10:49 | |
| *** henrynash has joined #openstack-keystone | 10:50 | |
| *** topol has joined #openstack-keystone | 10:50 | |
| openstackgerrit | henry-nash proposed a change to openstack/keystone: Ensure v3policysample correctly limits domain_admin access https://review.openstack.org/79897 | 10:55 |
| *** henrynash has quit IRC | 11:13 | |
| *** leseb has quit IRC | 11:19 | |
| *** leseb has joined #openstack-keystone | 11:20 | |
| *** leseb has quit IRC | 11:24 | |
| *** ChanServ changes topic to "test/gate jobs are queuing now in preparation for gerrit maintenance at 12:00 utc (eta to resume is 12:30 utc)" | 11:27 | |
| *** gokrokve has joined #openstack-keystone | 11:43 | |
| *** gokrokve has quit IRC | 11:47 | |
| openstackgerrit | sagar pradhan proposed a change to openstack/keystone: Bug 1230360 keystone-all --log-file logs to stdout https://review.openstack.org/79909 | 11:49 |
| *** leseb has joined #openstack-keystone | 12:20 | |
| *** leseb has quit IRC | 12:23 | |
| *** leseb has joined #openstack-keystone | 12:23 | |
| *** david-lyle has quit IRC | 12:24 | |
| *** ChanServ changes topic to "gerrit on review.openstack.org is down for maintenance (revised eta to resume is 13:00 utc)" | 12:24 | |
| *** saju_m has joined #openstack-keystone | 12:38 | |
| *** gokrokve has joined #openstack-keystone | 12:43 | |
| *** zhiyan_ is now known as zhiyan | 12:47 | |
| *** gokrokve has quit IRC | 12:48 | |
| *** openstackgerrit has quit IRC | 12:54 | |
| *** openstackgerrit has joined #openstack-keystone | 12:54 | |
| *** henrynash has joined #openstack-keystone | 13:01 | |
| *** openstackgerrit has quit IRC | 13:08 | |
| *** openstackgerrit has joined #openstack-keystone | 13:08 | |
| *** browne has joined #openstack-keystone | 13:22 | |
| ayoung | nkinder_, I just wrote this bug and assigned to you. https://bugs.launchpad.net/keystone/+bug/1291366 | 13:23 |
| *** haneef__ has joined #openstack-keystone | 13:25 | |
| *** haneef_ has quit IRC | 13:25 | |
| ayoung | actually, I assigned to me, but lets discuss | 13:26 |
| *** gokrokve has joined #openstack-keystone | 13:43 | |
| *** gokrokve has quit IRC | 13:48 | |
| *** wchrisj has joined #openstack-keystone | 13:51 | |
| openstackgerrit | henry-nash proposed a change to openstack/keystone: Ensure v3policysample correctly limits domain_admin access https://review.openstack.org/79897 | 13:54 |
| openstackgerrit | sagar pradhan proposed a change to openstack/keystone: Bug1273273 if run 'keystone-manage db_sync' without 'sudo', there should be a prompt that tables n ot created https://review.openstack.org/79950 | 13:55 |
| ayoung | henrynash, not sufficient | 13:56 |
| henrynash | ayoung: meaning... | 13:57 |
| ayoung | henrynash, the domain_id field on the user object needs to be immutable | 13:57 |
| henrynash | ayoung: Ok, so as I said I am going to put in a separate patch for that | 13:57 |
| ayoung | otherwise: a domain admin can create a user in their domain, then call update user... | 13:58 |
| ayoung | henrynash, how are you going to address it? | 13:58 |
| henrynash | ayoung: but without a role on that domain they can't autneticate | 13:58 |
| *** vhoward- has joined #openstack-keystone | 13:58 | |
| ayoung | something like a policy on the update API that checks the target value of the domain id? | 13:58 |
| henrynash | ayoung: so two ways we could address it... | 13:59 |
| henrynash | ayoung: 1) as you say, policy…but I can't work out how to write a rule that says "If the domain_id is included in the update, then go check it is the same as the one in the object you are updating" | 14:00 |
| ayoung | that sounds horrible and fragile. So I assume you have a better idea | 14:00 |
| henrynash | ayoung: as I say in the commit - I think we need to extend the policy engine to enable a check for existence of an attrinute | 14:00 |
| ayoung | henrynash, or we could just force domain_id to be immutable everywhere | 14:01 |
| henrynash | ayoung: I was going to add a config option ("domain_id_immuatble") which meant we reject updates to user/group/project if the domain_id is being changed | 14:01 |
| henrynash | ayoung: just like we do with id | 14:01 |
| ayoung | do we have a config option for id, or just force it? | 14:01 |
| henrynash | ayoung: no , we just force iyt | 14:02 |
| ayoung | same with domain_id, I think, then | 14:02 |
| henrynash | ayoung: my only concern was today you CAN change domain_id so we are changing functionality | 14:02 |
| ayoung | so what. We are patching a security hole | 14:02 |
| *** dims_ has quit IRC | 14:03 | |
| henrynash | ayoung: well, not if you use the standard policy file….since we don;t try and have domain admins etc. | 14:03 |
| henrynash | ayoung: there is no "hole" in that case | 14:03 |
| *** openstack has joined #openstack-keystone | 14:04 | |
| ayoung | nah, just one big hole that we throw everything in to | 14:04 |
| henrynash | ayoung: a pit, in fact | 14:04 |
| ayoung | "The PIT of DePAIR!": | 14:04 |
| henrynash | ayoung: I'll propose it with the config variable…we can always remove that if general few is we should outlaw this (of course witt multi-backend you DEFINITELY don;t want to be able to do this) | 14:05 |
| ayoung | henrynash, so....the policy file makes sense to me | 14:05 |
| ayoung | question is whether we need to clean up the syntax to make this more usable in a wider setting | 14:05 |
| henrynash | ayoung: I'll do it as a sparate patch. | 14:05 |
| henrynash | ayoung: so I tried to do that a bit…open to suggestions | 14:06 |
| ayoung | yeah...need to think about it. Might be something for the summit | 14:06 |
| ayoung | seems like a lot of that should be implicit | 14:06 |
| henrynash | ayoung: maybe…ok, I'll propose a second defect/patch for the domain_id immuatbility | 14:07 |
| ayoung | like: if I change something, I need perms for the pre and post states | 14:07 |
| ayoung | should not be something configurable...it should be rigidly enforced | 14:07 |
| ayoung | policy should be simply: you need this role to have the right permissions for this operation. | 14:08 |
| *** fungi has joined #openstack-keystone | 14:08 | |
| *** ChanServ sets mode: +o fungi | 14:08 | |
| ayoung | henrynash, this is why I like LDAP | 14:08 |
| *** fungi changes topic to "[ Icehouse RC blockers https://launchpad.net/keystone/+milestone/icehouse-rc1 ][ Icehouse RC Target Date: March 27th, 2014 ]" | 14:08 | |
| ayoung | ACLs are tough to master, but they make it really hard to expose info unintentionally | 14:08 |
| henrynash | ayoung: yeah, undestand | 14:08 |
| *** ChanServ sets mode: -o fungi | 14:08 | |
| *** fungi has left #openstack-keystone | 14:08 | |
| *** stevemar has joined #openstack-keystone | 14:09 | |
| openstackgerrit | David Stanek proposed a change to openstack/keystone: Cleanup keystoneclient tests https://review.openstack.org/79730 | 14:10 |
| openstackgerrit | David Stanek proposed a change to openstack/keystone: Cleanup fixture data added to test instances https://review.openstack.org/79729 | 14:10 |
| dolphm | lbragstad: just realized i meant nullable=False on https://bugs.launchpad.net/keystone/+bug/1272459 | 14:11 |
| *** daneyon has joined #openstack-keystone | 14:12 | |
| lbragstad | dolphm: ++ I was thinking that is what you meant | 14:12 |
| ayoung | dolphm, I think that the lock for synchronize revocation events is going to become a performance bottleneck | 14:13 |
| ayoung | I am running some client code and the difference between checking it each time and only synchronizing once a second is significant | 14:14 |
| dolphm | ayoung: i would imagine that to be the case | 14:15 |
| *** dims_ has joined #openstack-keystone | 14:16 | |
| ayoung | dolphm, I'm fairly certain that adding a delay in chacking will 1. break unit tests 2. make no real difference in security 3. sped things up | 14:16 |
| ayoung | question is if there is a different way to mitigate | 14:16 |
| ayoung | we can't assume that there is only one keystone server | 14:16 |
| ayoung | or that it isn't running in multiple workers, so there needs to be some interprocess synchronization | 14:16 |
| dolphm | ayoung: leave the delay configurable and advance the the clock in unit tests to trigger it when necessary | 14:17 |
| ayoung | I could make it a config value, set it to 0 for the unit tests, and have an explicit unit test for the cases where it is not 0 as well | 14:17 |
| ayoung | OK | 14:17 |
| dolphm | ayoung: and i thought it was configurable? | 14:17 |
| ayoung | dolphm, the delay? | 14:18 |
| ayoung | Just hacked it in last night | 14:18 |
| dolphm | ayoung: oh.. | 14:18 |
| dolphm | ayoung: let me look at the code, i guess i'm thinking of something else | 14:18 |
| ayoung | dolphm, that is the "last_fetched" | 14:18 |
| ayoung | so I can use that as the start of the sync delay | 14:19 |
| ayoung | dolphm, http://paste.openstack.org/show/73252/ | 14:19 |
| *** chandan_kumar has quit IRC | 14:22 | |
| openstackgerrit | sagar pradhan proposed a change to openstack/keystone: keystone-all --log-file logs to stdout https://review.openstack.org/79909 | 14:22 |
| dolphm | ayoung: makes sense, although i feel like it should read self._last_fetch < timeutils.utcnow() - delta ? | 14:23 |
| ayoung | dolphm, sure. | 14:24 |
| *** chandan_kumar has joined #openstack-keystone | 14:25 | |
| ayoung | dolphm, CONF.revoke.fetch_delay ? | 14:25 |
| dolphm | ayoung: for the delta? | 14:25 |
| *** gokrokve has joined #openstack-keystone | 14:25 | |
| ayoung | yeah | 14:26 |
| dolphm | ayoung: from a deployer perspective, your configuring the duration of cache validity -- it only happens to be implemented as a sort of "delay" | 14:26 |
| ayoung | dolphm, CONF.revoke.cached_duration ? | 14:27 |
| *** browne has quit IRC | 14:27 | |
| ayoung | dolphm, CONF.revoke.cache_duration ? | 14:27 |
| dolphm | ayoung: i'd make it look like all the other dogpile config, and then eventually replace the lock, etc, with a one liner call to dogpile | 14:27 |
| ayoung | ah | 14:27 |
| ayoung | don't want to pickle the tree each call | 14:28 |
| dolphm | ayoung: that's why there are different backends for dogpile | 14:28 |
| *** cmart has joined #openstack-keystone | 14:30 | |
| *** david-lyle has joined #openstack-keystone | 14:30 | |
| ayoung | dolphm, OK, makes sense. | 14:31 |
| *** browne has joined #openstack-keystone | 14:32 | |
| ayoung | cmart, ask in here....Private messages mean that the smarter people can;t respond | 14:34 |
| cmart | Oh I see... OK | 14:35 |
| cmart | I put some comments on the whiteboard related to the bp: https://blueprints.launchpad.net/keystone/+spec/tenant- | 14:35 |
| dolphm | henrynash: why is https://bugs.launchpad.net/keystone/+bug/1291393 targeting RC1? i don't want to introduce breaking changes right now | 14:35 |
| cmart | I'm trying to dig into the code and see what things we should consider for the bp. If you can take a look at them, that would be great | 14:35 |
| cmart | me either, but I'm just trying to understand the working items that ayoung mentioned in that bp | 14:36 |
| cmart | that's all | 14:36 |
| cmart | I'm new with Keystone and your feedback about this is imperative for me :) | 14:36 |
| dolphm | henrynash: looks like wishlist + juno to me | 14:36 |
| henrynash | dolphm: so my place was to use a config variable domain_id_immutable that would be set to False by default (i.e. no change to the existing behaviour), but could be set to True to harder | 14:37 |
| ayoung | https://blueprints.launchpad.net/keystone/+spec/tenant-expiration-dates | 14:37 |
| henrynash | harden | 14:37 |
| ayoung | cmart, its the wrong approach | 14:37 |
| cmart | ayoung, I'm listening (reading) :) | 14:37 |
| henrynash | dolphm: and for Juno we might state that this switched two be True by default | 14:37 |
| ayoung | you want to have the whole thing managed outside of Keystone, via Heat, if the goal is to have " a limited time offer" | 14:38 |
| *** topol has quit IRC | 14:38 | |
| ayoung | you are going to have to make a decisions "ok, times up, do we delete all of your resources, or just disable them" | 14:38 |
| dolphm | henrynash: approach sounds good, but still sounds like wishlist + juno to me | 14:38 |
| ayoung | I mean, the specific feature, sure, sounds good | 14:39 |
| stevemar | dolphm, question about revoking token based on idp deletion | 14:39 |
| henrynash | so this came out of bug #1287219 when I discovered it could not fix this by policy alone | 14:40 |
| ayoung | but the general "we want to offer you a limited term freebee, and then you pay to extend" requires integration, and we should not be building keystone around that model. Not that Keystone shouldn't support it | 14:40 |
| *** chandankumar_ has joined #openstack-keystone | 14:40 | |
| dolphm | cmart: keystone already exposes all the machinery (project enable/disabled), immediate token revocation, etc, that you need to take advantage of... i don't see much of a need to support "expiration dates" inside keystone itself | 14:40 |
| *** chandankumar_ has quit IRC | 14:40 | |
| henrynash | dolphm: I think ideally it would be with getting the change into rc1 (disabled by config) since it closes a potential nasty hole in our v3 domain admin policy protection | 14:42 |
| henrynash | dolphm: sorry for poor typing….multi-tasking | 14:42 |
| stevemar | dolphm, if we used revocation extension to delete federation tokens, aren't we depending on something that is optional? | 14:43 |
| bknudson | henrynash: it would be a security vulnerability if it wasn't fixed? | 14:43 |
| henrynash | bknudson: a potential one…..if you are using the v3 policy to create a domain admin scope of use | 14:44 |
| cmart | dolphm, that idea came from a discussion that we have with ayoung in the mailing list. (http://markmail.org/message/asrlobgh3ajqju7b) | 14:45 |
| cmart | if that's OK for you, we can discuss it.. | 14:46 |
| dolphm | henrynash: if that's how you're approaching it, then definitely document that in the bug | 14:50 |
| dolphm | cmart: yes, i saw the conversation when it occurred | 14:50 |
| *** thedodd has joined #openstack-keystone | 14:50 | |
| dolphm | henrynash: (specifically, how to exploit the "security hole" -- otherwise it just reads as a wishlist bug) | 14:52 |
| cmart | ayoung, So your saying that we should support the functionality but not build it around Keystone, Is that it? I'm not a native speaker, so there are couple of things of what you write that I don't quite understand. Sorry for that | 14:52 |
| *** andreaf has quit IRC | 14:52 | |
| dolphm | cmart: i'm a native speaker and i don't understand half of what ayoung writes ;) | 14:52 |
| marekd | dolphm: :D | 14:53 |
| henrynash | dolphm: will add | 14:53 |
| ayoung | cmart, I'm saying that "project start times and end times" by itself is a reasonable feature. What I am also saying is that the "limited time offer" functionality that someone said that they wanted it for (don't think it was you, don't remember) will require a lot more external work | 14:54 |
| ayoung | so I'd be hesitant to put too much effort into something that was "neither necessary nor sufficient" for the requirement | 14:54 |
| *** casanch1 has joined #openstack-keystone | 14:54 | |
| ayoung | dolphm, it is because I am a fancy-shmancy northeast intellectual and y'all are a no-nonsense, straight-talking Texan. | 14:57 |
| dolphm | lbragstad: morganfainberg_Z: what's with the change to migration 37? :( https://review.openstack.org/#/c/79159/ | 14:58 |
| dolphm | ayoung: +++ | 14:58 |
| lbragstad | dolphm: I think morganfainberg_Z added that in place of the downgrade. | 14:59 |
| dolphm | lbragstad: oh weird... | 15:00 |
| lbragstad | dolphm: the way I had it written originally, it pretty much just reverted the upgrade and if we had an sql.exc.IntegrityError because the descriptions of two regions weren't unique, I just made a new description with the uuid and the original description | 15:01 |
| *** marcoemorais has joined #openstack-keystone | 15:01 | |
| *** marcoemorais has quit IRC | 15:01 | |
| lbragstad | dolphm: I *think* morganfainberg_Z was coming at it from the vantage point that we didn't want to reintroduce the problem on downgrade. | 15:01 |
| *** gokrokve has quit IRC | 15:02 | |
| *** gokrokve has joined #openstack-keystone | 15:03 | |
| dolphm | lbragstad: i would have done it like you had it originally... but i think i understand what's going on now | 15:03 |
| *** saju_m has quit IRC | 15:03 | |
| cmart | ayoung, that someone is casanch1, he just connected to the room.. and the module that will use the functionaliy will be Blazar (ex-Climate) | 15:03 |
| lbragstad | dolphm: ok, good deal. morganfainberg_Z will probably be able to explain his reasoning a little better than I did. :) | 15:04 |
| *** saju_m has joined #openstack-keystone | 15:04 | |
| *** gokrokve_ has joined #openstack-keystone | 15:04 | |
| dolphm | lbragstad: i just auto -1'd based on the change to 37, but i'm glad i asked! | 15:05 |
| marekd | dstanek: o/ I didn't know testing data is loaded always before executing each test. Good to know that. I removed that questionable line that was reverting old mapping. | 15:05 |
| cmart | ayoung, but I'm not getting your point.. I never talk about limited time offer.. I'm justr trying to get feedback and implementation hints related to the bp.. | 15:05 |
| lbragstad | dolphm: ++ | 15:06 |
| cmart | maybe I'm confused, I don't know :S | 15:07 |
| dolphm | lbragstad: so old37 -> 43 -> 36 -> new37 -> 43 looks like it's handled, but not tested (not sure how we would test though) | 15:07 |
| *** richm has joined #openstack-keystone | 15:08 | |
| *** gokrokve has quit IRC | 15:08 | |
| lbragstad | dolphm: in the test I suppse we could mock out the behavior of the old37 | 15:08 |
| lbragstad | and create a region table with description unique=True and nullable=False and use that as our old37 migration? | 15:09 |
| bknudson | why is it necessary to change the migration? | 15:10 |
| lbragstad | bknudson: morganfainberg_Z added the change to migration 37 to work around introducing the unique=True problem on downgrade. | 15:11 |
| lbragstad | bknudson: he has a comment on downgrade() in migration 043, | 15:11 |
| * lbragstad makes a note to add a tempest test for region description uniqueness | 15:12 | |
| bknudson | I don't think migrations should work that way | 15:12 |
| openstackgerrit | Marek Denis proposed a change to openstack/keystone: Validate groups presence for federated authn https://review.openstack.org/79284 | 15:12 |
| *** topol has joined #openstack-keystone | 15:13 | |
| lbragstad | bknudson: originally, I just stuffed everything for region.description back into the region table with unique=True on the description column | 15:13 |
| bknudson | lbragstad: that sounds like the right way to do it. | 15:15 |
| lbragstad | bknudson: if you want to leave a comment on the review, and we can try and sync up with morganfainberg_Z a little later when he's available? | 15:16 |
| bknudson | lbragstad: I left a comment on the review. | 15:16 |
| lbragstad | bknudson: thanks | 15:16 |
| bknudson | lbragstad: you're the owner of the review. | 15:17 |
| lbragstad | bknudson: yep | 15:17 |
| *** topol has quit IRC | 15:18 | |
| cmart | ayoung, Are you there? | 15:18 |
| *** henrynash has quit IRC | 15:20 | |
| ayoung | cmart, as I said, it was notyou that asked about it. Just that was one justification for this feature. | 15:21 |
| *** leseb has quit IRC | 15:23 | |
| *** chandan_kumar has quit IRC | 15:26 | |
| *** chandan_kumar has joined #openstack-keystone | 15:28 | |
| *** chandan_kumar has quit IRC | 15:28 | |
| *** browne has quit IRC | 15:28 | |
| *** gyee has joined #openstack-keystone | 15:31 | |
| dolphm | ayoung: the one bug you have already have a patch for, and you don't even assign it to yourself? :P https://bugs.launchpad.net/keystone/+bug/1291423 | 15:35 |
| ayoung | dolphm, nope | 15:35 |
| ayoung | dolphm, I am going to let morganfainberg_Z deal with it, since you said it was a dogpile issue. :) | 15:35 |
| cmart | ayoung, What do you suggest about the bp? Should we continue the analysis? What could be the next steps around that? | 15:37 |
| ayoung | cmart, why do you want the feature? | 15:39 |
| *** browne has joined #openstack-keystone | 15:39 | |
| *** saju_m has quit IRC | 15:40 | |
| dolphm | ayoung: for https://launchpad.net/climate | 15:41 |
| cmart | ayoung, we have a bp on Blazar(https://blueprints.launchpad.net/climate/+spec/tenant-reservation-concept) in which the idea is manage all the leases associated to one tenant.. | 15:42 |
| dolphm | cmart: and what about that use case can you not implement in climate? what is the critical piece that's missing from keystone? | 15:44 |
| *** browne has quit IRC | 15:44 | |
| cmart | well,. there's no critical piece missing in Keystone | 15:45 |
| cmart | but since this information was needed, we thought that it could be stored in Keystone | 15:45 |
| cmart | and moreover, that could be useful for keystone without having climate or any other reservation component in the picture | 15:45 |
| cmart | this could be done in climate itself, but it made sense (at least for me) to store the tenant information in Keystone | 15:46 |
| ayoung | cmart, first off, they are projects, not tenants. We're trying to clean up that language. | 15:47 |
| cmart | yes, my bad | 15:47 |
| ayoung | But more important: a feature for only one use case is risky | 15:47 |
| ayoung | Keystone is a general purpose service, and lots of people have proposed storing stuff in there that doesn't belong | 15:48 |
| ayoung | for example, Horizon preferences for UI and such | 15:48 |
| cmart | so, do you think that having project start and end dates in Keystone is not a good idea? | 15:49 |
| ayoung | right now we have enabled/disabled. Seems like it is a business rule to switch that to date based, and I'm not convinced | 15:49 |
| ayoung | I'm not saying no, just that I have not seen enough to get me to yes | 15:50 |
| *** browne has joined #openstack-keystone | 15:50 | |
| cmart | well... this can be implemented only in climate and store those start and end dates in that db, that can be done for sure | 15:51 |
| ayoung | cmart, see if other projects have a need for the same feature, and build the case for it if you really want it. There is no rush, and nothing that can't be postponed. | 15:52 |
| ayoung | one benefit to keeping in climate is that it would work with older versions of Keystone | 15:53 |
| cmart | ok, I will do that | 15:53 |
| cmart | and will switch to work on climate for this | 15:53 |
| cmart | thanks for all the feedback! | 15:54 |
| cmart | and sorry for the bad english!! | 15:55 |
| *** gyee has quit IRC | 15:59 | |
| *** leseb has joined #openstack-keystone | 16:01 | |
| *** henrynash has joined #openstack-keystone | 16:07 | |
| *** daneyon has quit IRC | 16:12 | |
| *** daneyon has joined #openstack-keystone | 16:12 | |
| *** gordc has joined #openstack-keystone | 16:19 | |
| gordc | stevemar: ping | 16:19 |
| *** dstanek_afk has joined #openstack-keystone | 16:19 | |
| *** dstanek has quit IRC | 16:20 | |
| *** dstanek_afk has quit IRC | 16:24 | |
| *** gokrokve_ has quit IRC | 16:25 | |
| *** devlaps has joined #openstack-keystone | 16:26 | |
| *** jaosorior has quit IRC | 16:30 | |
| stevemar | gordc, pong | 16:33 |
| gordc | stevemar: can you connect to the lab? | 16:34 |
| stevemar | gordc, nope | 16:34 |
| stevemar | gordc, it started before the scrum, booo | 16:35 |
| gordc | stevemar: cool cool. time to relax. | 16:35 |
| *** dstanek_afk has joined #openstack-keystone | 16:35 | |
| gordc | stevemar: yeah, i had to dial in with cell. my ip phone went down too. | 16:36 |
| stevemar | gordc, yeah, just verified with another person, it's down for them too | 16:39 |
| gordc | stevemar: wicked. | 16:39 |
| *** amcrn has joined #openstack-keystone | 16:42 | |
| *** gyee has joined #openstack-keystone | 16:49 | |
| *** gokrokve has joined #openstack-keystone | 16:50 | |
| *** marcoemorais has joined #openstack-keystone | 17:02 | |
| openstackgerrit | Marek Denis proposed a change to openstack/keystone: Validate groups presence for federated authn https://review.openstack.org/79284 | 17:03 |
| *** harlowja_away is now known as harlowja | 17:04 | |
| stevemar | gordc, it works again | 17:04 |
| gordc | gordc: cool cool. perfect time to grab lunch. :) | 17:06 |
| *** mpetason has joined #openstack-keystone | 17:09 | |
| *** gokrokve_ has joined #openstack-keystone | 17:09 | |
| *** andreaf has joined #openstack-keystone | 17:11 | |
| *** marcoemorais has quit IRC | 17:11 | |
| *** henrynash has quit IRC | 17:11 | |
| *** gokrokve has quit IRC | 17:12 | |
| *** henrynash has joined #openstack-keystone | 17:13 | |
| *** henrynash has quit IRC | 17:13 | |
| *** morganfainberg_Z is now known as morganfainberg | 17:13 | |
| *** marcoemorais has joined #openstack-keystone | 17:13 | |
| *** casanch1 has quit IRC | 17:15 | |
| *** casanch1_ has joined #openstack-keystone | 17:16 | |
| *** casanch1_ is now known as casanch1 | 17:16 | |
| *** saju_m has joined #openstack-keystone | 17:17 | |
| morganfainberg | bknudson, hi | 17:17 |
| bknudson | morganfainberg: hi | 17:17 |
| morganfainberg | lbragstad, bknudson, what is the issue with the downgrade issue? | 17:17 |
| *** casanch1 has left #openstack-keystone | 17:18 | |
| bknudson | morganfainberg: we can't change existing migrations | 17:18 |
| morganfainberg | bknudson, yes we can in this case. | 17:18 |
| bknudson | morganfainberg: otherwise we could have different versions of the database out there. | 17:18 |
| bknudson | I don't want to have to deal with that. | 17:18 |
| morganfainberg | if you look at the code we will always end up with the same version of the db | 17:18 |
| bknudson | morganfainberg: not at level 37 | 17:19 |
| morganfainberg | ok, i take that back, if you migrate ½ way thought i and never move, yes you have a different version | 17:19 |
| morganfainberg | is that _really_ a case we expect? | 17:19 |
| bknudson | you could have one system is 37 is unique=True and another is 37 and unique=False | 17:20 |
| lbragstad | dolphm: discussing migrations | 17:20 |
| lbragstad | 037 and 043, since we were talking about htat earlier | 17:20 |
| dolphm | morganfainberg: lbragstad: o/ | 17:20 |
| morganfainberg | then remove the extra logic | 17:20 |
| morganfainberg | it is, imnsho, really dumb to migrate to a unique index'd table then respin to a non-unique indexed table within the same release. but if we can't do it | 17:21 |
| morganfainberg | remove that. | 17:21 |
| bknudson | it's unfortunate, but we can't do it. | 17:22 |
| morganfainberg | if we are doing this we can't do the buffer for backport, it's the same argument | 17:22 |
| morganfainberg | well we shouldn't | 17:22 |
| bknudson | it would only be able to be used in the most extreme situation | 17:22 |
| morganfainberg | bknudson, i believe that with idempotent changes we should be able to do this within a release | 17:22 |
| morganfainberg | and yes, i'm willing to argue that w/ the TC if needed. | 17:23 |
| morganfainberg | bridging across releases, no | 17:23 |
| dolphm | morganfainberg: i agree (and i don't know why anyone would want to continuously deploy keystone) but people claim to do so | 17:23 |
| morganfainberg | even w/ CD this would have a net positive result | 17:24 |
| dolphm | morganfainberg: the last time we rewrote any migration history was during grizzly development i believe, and we got a complaint or two about it | 17:24 |
| dolphm | morganfainberg: your change is graceful compared to what we did in grizzly | 17:24 |
| morganfainberg | dolphm, i'll argue this one | 17:25 |
| dolphm | which was simply to fix a broken migration without writing a new one | 17:25 |
| morganfainberg | dolphm, but if the answer is no, just pull the code out | 17:25 |
| dolphm | morganfainberg: personally i'm comfortable with your change and ready to +2 | 17:25 |
| morganfainberg | adding in a unique contraint back in is easy, though we don't want to reverse the migration, just apply a unique index | 17:25 |
| morganfainberg | dolphm, like i said, i'm happy to back down, but if you need arguing, i will :) | 17:25 |
| dolphm | morganfainberg: ... which might crash | 17:26 |
| bknudson | re-applying the unique index is probably going to crash | 17:26 |
| morganfainberg | bknudson, ? | 17:26 |
| dolphm | bknudson: ++ | 17:26 |
| morganfainberg | oh crud didn't even realize i worked around that | 17:26 |
| bknudson | morganfainberg: expect a downgrade that adds the unique index to not work. | 17:27 |
| morganfainberg | :P | 17:27 |
| bknudson | especially for a field like description that I wouldn't expect to be unique. | 17:27 |
| morganfainberg | it just made sense to never re-add the unique index. | 17:27 |
| morganfainberg | derp. | 17:27 |
| * dolphm +2'd but didn't approve | 17:27 | |
| morganfainberg | bknudson, ++ | 17:27 |
| morganfainberg | so ... how would you conflict resolve? | 17:27 |
| morganfainberg | can't drop regions | 17:27 |
| morganfainberg | just add some garbage into the description? | 17:28 |
| bknudson | morganfainberg: you don't. You fail and complain that we can't downgrade. | 17:28 |
| morganfainberg | bknudson, i vote against that. | 17:28 |
| morganfainberg | bknudson, personally. | 17:28 |
| morganfainberg | it means if you upgrade to I you can't go back to H | 17:28 |
| morganfainberg | it's a restore from backup | 17:28 |
| bknudson | you can go back. You have to resolve the migration problem yourself. | 17:28 |
| morganfainberg | precluding running the service that is | 17:28 |
| bknudson | you can add garbage to your descriptions if that's what you want. | 17:29 |
| morganfainberg | eh i guess my usecase is the edge | 17:29 |
| morganfainberg | well, i'm content to let it crash and raise an error if that is what is needed | 17:29 |
| morganfainberg | s/crash and raise/make a pretty error that says you can't do this. | 17:30 |
| dolphm | bknudson: why not just have a more stable migration path? | 17:30 |
| lbragstad | wouldn't you be able to use the existing description and hte uuid of the region to make a unique description if you do have a conflict? | 17:30 |
| bknudson | dolphm: I'm worried that we've got a database schema that people might have been running and testing with since migration 37 ... vs a new change that we haven't been running with | 17:31 |
| bknudson | dolphm: and, we'll have a bunch of situations that we've never tested with, such as the ones that are 37-41-before-this and 37-41-after this. | 17:32 |
| bknudson | oops, should be 37-43 | 17:32 |
| dolphm | bknudson: i believe morganfainberg's patch handles those situations really well | 17:32 |
| *** cmart has quit IRC | 17:33 | |
| dolphm | i'd argue that migration 37 contains a bug, which morgan's patch fixes | 17:33 |
| dolphm | if you were subject to the buggy version of the migration, migration 43 will fix your schema | 17:33 |
| dolphm | and the bug never sees a proper stable release | 17:33 |
| morganfainberg | dolphm, ++ that is my arguemnt | 17:33 |
| morganfainberg | if this was across releases i would -2 this change | 17:34 |
| morganfainberg | e.g. H to I | 17:34 |
| morganfainberg | i can't make a solid argument for that. | 17:34 |
| bknudson | why don't we just squash all the migrations down to 1? | 17:34 |
| dolphm | fwiw, there is an accepted precedence in the community for correcting bugged migrations, even when they're old | 17:35 |
| bknudson | I guess I just don't see how bad it is to keep the migration as it was? | 17:36 |
| morganfainberg | dolphm, bknudson, it's a quick respin to solve the rewrite history issue, but i still think 037 is bugged | 17:36 |
| dolphm | bknudson: would you accept the argument that morganfainberg's version of the patch is less risky since there's less code to go wrong? (vs patchset 1) | 17:36 |
| bknudson | there's not much code in patch set 1. | 17:37 |
| dolphm | bknudson: patchset 4* | 17:38 |
| bknudson | well, patch set 1 is broken anyways since it has hardcoded sql. | 17:38 |
| morganfainberg | bknudson, and i'll fix that in J | 17:38 |
| morganfainberg | we'll start at 036 | 17:38 |
| *** openstackstatus has quit IRC | 17:38 | |
| morganfainberg | but. anyway | 17:38 |
| *** openstackstatus has joined #openstack-keystone | 17:38 | |
| * bknudson is in a meeting for a while | 17:39 | |
| morganfainberg | dolphm, i feel like we have precedent in either case. | 17:39 |
| bknudson | dolphm: where's an example where we changed an existing migration? | 17:40 |
| morganfainberg | dolphm, so, i'm good with either solution. I am positive fixing the bug is the right way, but it's not worth massive quibbling over (we have plenty else to work on) | 17:40 |
| morganfainberg | jamielennox|away, did you need me to fix the kite repo stuff for you? | 17:41 |
| bknudson | if we change a migration and it causes any kind of problem there are going to be a lot of questions asked. | 17:41 |
| morganfainberg | jamielennox|away, i'm happy to do so, just let me know | 17:41 |
| bknudson | we'll be banished or even shunned. | 17:41 |
| dolphm | bknudson: (i assume you mean where it's documented as an acceptable practice) | 17:43 |
| dolphm | i ran into this, but it's entirely written with regard to running migrations online https://wiki.openstack.org/wiki/DBMigrationBestPractices | 17:44 |
| bknudson | dolphm: yes, just seeing what's an acceptable reason to change a migration. | 17:44 |
| morganfainberg | oh live migrations would be awesome. | 17:44 |
| morganfainberg | but... uhhh | 17:44 |
| dolphm | lol | 17:44 |
| bknudson | not something we've tried to achieve | 17:45 |
| dolphm | i'd be happy to do live migrations if someone wants to properly test us and gate us against them | 17:45 |
| bknudson | although it should work in this case. | 17:45 |
| dolphm | until then, i have zero interest in pretending | 17:45 |
| morganfainberg | dolphm, i would love to do that. but before we do that we have some other things to deal with. | 17:45 |
| morganfainberg | dolphm, and i bet it would take us 2+ releases to get all the scaffolding in to properly make it happen. | 17:46 |
| morganfainberg | let alone tempest etc. | 17:46 |
| dolphm | yeah | 17:46 |
| * morganfainberg sees using something like a protobuf as the core data mechanism within keystone to get there. | 17:47 | |
| morganfainberg | then db is just persistence, pull it all apart and put it back together. | 17:47 |
| morganfainberg | not that protobuf is the tech to use. | 17:47 |
| morganfainberg | but something kindof like it | 17:47 |
| stevemar | can i get another review for https://review.openstack.org/#/c/73031/ it's already passed the blk-u test | 17:48 |
| morganfainberg | stevemar, oh i need to find a -1 reason now :P | 17:49 |
| morganfainberg | stevemar, sure. will look at it in a few | 17:49 |
| stevemar | morganfainberg, btw - your comment "but I want to do one more pass on it vs keystone's oauth1 extension" | 17:49 |
| stevemar | morganfainberg, 'splain? did you want me to write a python script to test it out or something? | 17:49 |
| morganfainberg | dolphm, so, lets just make a call on the migration, i think we've already spent more time than it warrants | 17:50 |
| dolphm | stevemar: morganfainberg: approvaled stevemar's change | 17:50 |
| stevemar | yahoo | 17:50 |
| morganfainberg | stevemar, nah, i just didn't want to +2 w/o having a sec to revisit keystone oauth1 in my head so i saw how ksc played w/ it | 17:50 |
| stevemar | morganfainberg, ah okay | 17:50 |
| dolphm | morganfainberg: so, the specific complaint i got last time (sorry for not being able to cite bknudson, i gave up searching)... | 17:51 |
| *** Fin1te has joined #openstack-keystone | 17:51 | |
| dolphm | morganfainberg: was that we fixed a bug in a migration while leaving those chasing master in the dust (with a broken schema) | 17:51 |
| morganfainberg | stevemar, i was brain fried. nothing looked wrong but since it was late and all, didn't feel good +2/+A | 17:51 |
| stevemar | morganfainberg, i might make one anyway, i saw ayoung do that in https://review.openstack.org/#/c/79096/1/examples/scripts/exercise_v3_regions.py and thought it was a good idea | 17:51 |
| morganfainberg | dolphm, ah, yeah | 17:51 |
| dolphm | morganfainberg: this obviously isn't that scenario, so +2 | 17:51 |
| morganfainberg | dolphm, and i stand by if we were across stable releases this kind of stuff couldn't fly | 17:51 |
| morganfainberg | barring a MASSIVe bug that needed a backport | 17:52 |
| ayoung | stevemar, oh yea | 17:52 |
| *** mpetason has left #openstack-keystone | 17:52 | |
| ayoung | stevemar, I have a better version of that coming up | 17:52 |
| dolphm | morganfainberg: yeah, it'd have to be more broken than this to warrant this type of approach | 17:52 |
| morganfainberg | a unique constrained column != massive | 17:52 |
| morganfainberg | dolphm, sounds good, let me know if we need to respin to downgrade and add the unique index constraint back in | 17:52 |
| morganfainberg | dolphm, otherwise i'll leave it (though this does need to land in I imo) | 17:53 |
| dolphm | morganfainberg: going to cite IRC logs here and +A | 17:53 |
| morganfainberg | dolphm, ++ | 17:53 |
| marekd | dolphm: stevemar ayoung bknudson dstanek_afk lbragstad: Gents, another round of: https://review.openstack.org/#/c/79284/? | 17:53 |
| *** dstanek_afk is now known as dstanek | 17:53 | |
| morganfainberg | marekd, i almost feel left out :P | 17:53 |
| morganfainberg | marekd, >.> | 17:53 |
| ayoung | marekd, -2. Now stop bothering me. Just kidding. Looks good. | 17:54 |
| marekd | morganfainberg: especially for ya: another round of https://review.openstack.org/#/c/79284/ ? :-) | 17:54 |
| morganfainberg | dolphm, for the test config stuff, lets plan for that to hit J1 during the "quiet" time. | 17:54 |
| morganfainberg | so the massive test restructuring doesn't get in the way, and it gets us a lot closer to parallel testing | 17:55 |
| morganfainberg | dstanek, ^ (for my changes) | 17:55 |
| ayoung | marekd, once again, though, are you really really absotutapositicompletly sure you want this? | 17:55 |
| morganfainberg | dstanek, so, lets chase yours in first since i think they will provide more immidiate benefit | 17:55 |
| dolphm | morganfainberg: lbragstad: bknudson: approved | 17:56 |
| dolphm | morganfainberg: dstanek's patch? | 17:56 |
| marekd | ayoung: i think we got through this yesterday and it's not only my opinoin that this could be useful... | 17:56 |
| dolphm | morganfainberg: adopt oslotest j1 day 1? | 17:56 |
| morganfainberg | dolphm, no mine, i have a chain that rips out most test configs | 17:57 |
| dolphm | morganfainberg: link? | 17:57 |
| dstanek | morganfainberg: chase which one? | 17:57 |
| morganfainberg | dolphm, https://review.openstack.org/#/c/79524/ https://review.openstack.org/#/c/79525/ https://review.openstack.org/#/c/79526/ | 17:57 |
| marekd | ayoung: i guess there are also other cores who think this should be landed, so you can let them decide :-) | 17:57 |
| dolphm | dstanek: ooh, we should hold your oslotest series for j1 anyway, given it introduces a new dep post-freeze | 17:57 |
| marekd | ayoung: just please don't -2 it :-) | 17:57 |
| morganfainberg | dolphm, oh his whole change is oslotest isn't it | 17:57 |
| morganfainberg | dstanek, doh! i was aiming for yours to land since they would conflict with my chain | 17:58 |
| morganfainberg | dolphm, ^ | 17:58 |
| morganfainberg | dolphm, dstanek, then land the massive test config changes in J1. | 17:58 |
| dstanek | dolphm, morganfainberg: i can rebase most of my memory patches on current master instead of the oslstest change | 17:58 |
| dolphm | marekd: IIRC, you satisfied ayoung's objection already in the current patchset | 17:58 |
| marekd | dolphm: yes | 17:58 |
| dolphm | dstanek: that'd be handy :) | 17:59 |
| morganfainberg | marekd, if everyone else is on board with this, i'm willing to give it a go. | 17:59 |
| ayoung | marekd, I won't -2. But I do think that it is a mistake | 17:59 |
| morganfainberg | marekd, i am not sure i like the solution (might be more in ayoung's camp) but i am unsure if there is a better one readily available | 17:59 |
| morganfainberg | or that we could land before I ships | 18:00 |
| morganfainberg | or if we even need this. | 18:00 |
| ayoung | I think that somoene is going to want to use Federation as intended, and won't care that the groups aren't in identity, and will go to fix it and be tripped up by this change and reverse it | 18:00 |
| morganfainberg | i _think_ forcing the use of identity in this case is probably wrong. | 18:00 |
| morganfainberg | but i can see why there is a desire to control the groups | 18:01 |
| ayoung | morganfainberg, marekd let me ask the real question | 18:01 |
| ayoung | if I don;t have anything in Identity, will SAML still work today? | 18:01 |
| dstanek | dolphm: shouldn't take long, just a little wierd because there will be a dependency on a class that doesn't exist | 18:01 |
| ayoung | no users, no groups, just data from the SAML assertion | 18:01 |
| ayoung | ? | 18:01 |
| ayoung | and the mapping, of course? | 18:02 |
| morganfainberg | dstanek, ++ I would like to land your fixes in I. | 18:02 |
| marekd | ayoung: so if you are okay with "id don't care" approach i'd recommend check groups and only log a warning, something that may help admins debug their configs...something i proposed at the beginning and you wanted to return HTTP 500, which has a real impact on that... | 18:02 |
| morganfainberg | dstanek, we can do the oslotest and the config destroying in J | 18:02 |
| *** marcoemorais has quit IRC | 18:02 | |
| *** marcoemorais has joined #openstack-keystone | 18:03 | |
| dstanek | morganfainberg: it would be nice to run the full test suite while i am working on bugs | 18:03 |
| marekd | if i was to decide i'd leave warnings instead of raising HTTP 500 errors. | 18:03 |
| ayoung | marekd, I need an answer to my question before we can move on. Will it work? | 18:03 |
| dolphm | ayoung: no it won't work at all | 18:04 |
| *** marcoemorais has quit IRC | 18:04 | |
| dstanek | morganfainberg: what's funny is i'm going to rebase these commits and i can't properly test the first few because i'll run out of memory | 18:04 |
| *** marcoemorais has joined #openstack-keystone | 18:04 | |
| dolphm | ayoung: you'd be able to generate unscoped tokens i believe, but you'd be unable to acquire any sort of authorization as there's no authorization to map to | 18:04 |
| morganfainberg | dstanek, lol | 18:04 |
| marekd | dolphm: ++ | 18:05 |
| ayoung | dolphm, marekd ok, ignoring this change, what would it take to make it work? | 18:05 |
| marekd | ayoung: depends what you want, map saml2 assertion directly to the rules? | 18:05 |
| marekd | blah | 18:06 |
| marekd | s/rules/roles | 18:06 |
| dolphm | ayoung: federation is currently intended to provide a authentication frontend to our existing authorization infrastructure; i'd love to see federation completely replace that entire authorization infrastructure all the way through auth_token and oslo policy, but that's an *extensive* redesign that we simply did not pursue during icehouse as it would be completely incompatible with everything about existing keystone de | 18:06 |
| dolphm | ployments | 18:06 |
| ayoung | dolphm, OK. So with that understanding, I'm ok with marekd 's change. But it will be yet another thing to work around when the time comes to do the reworking. | 18:07 |
| ayoung | marekd, the change itself looks good | 18:07 |
| morganfainberg | dolphm, that is a good description that clarifies these needs | 18:07 |
| dolphm | ayoung: all this code would be out of the picture -- you wouldn't have to contact keystone at all | 18:07 |
| marekd | ayoung: i need to run in 15 minutes, can we talk about that 'redesign' thing tomorrow? | 18:07 |
| ayoung | marekd, hows about at the summit instead? | 18:08 |
| marekd | ayoung: ah, sure! | 18:08 |
| dolphm | ayoung: +++ | 18:08 |
| marekd | ayoung: thought you wanted to do it right about now | 18:08 |
| ayoung | nope | 18:08 |
| morganfainberg | dstanek, want me to run the tests? i can do it | 18:08 |
| marekd | ayoung: worth proposing a design session ? | 18:08 |
| ayoung | just making sure we are all on the same beat | 18:08 |
| ayoung | marekd, thought I already did? | 18:08 |
| marekd | ayoung: http://summit.openstack.org/cfp/details/6 ? | 18:09 |
| ayoung | marekd, http://summit.openstack.org/cfp/details/7 | 18:09 |
| morganfainberg | marekd, yes the code looks good to me | 18:09 |
| marekd | morganfainberg: thanks. | 18:09 |
| ayoung | 6 is for the Id thing that henrynash was working on...but we have quite a few sessions already, so maybe they will get merged | 18:10 |
| openstackgerrit | David Stanek proposed a change to openstack/keystone: Very minor cleanup to default_fixtures https://review.openstack.org/80040 | 18:10 |
| openstackgerrit | David Stanek proposed a change to openstack/keystone: Cleanup backends after each test https://review.openstack.org/79726 | 18:10 |
| openstackgerrit | David Stanek proposed a change to openstack/keystone: Cleanup of instance attrs in core tests https://review.openstack.org/79727 | 18:10 |
| openstackgerrit | David Stanek proposed a change to openstack/keystone: Cleanup keystoneclient tests https://review.openstack.org/79730 | 18:10 |
| openstackgerrit | David Stanek proposed a change to openstack/keystone: Cleanup fixture data added to test instances https://review.openstack.org/79729 | 18:10 |
| morganfainberg | oh is summit sessions open? | 18:10 |
| openstackgerrit | David Stanek proposed a change to openstack/keystone: Cleans up test data from limit tests https://review.openstack.org/79728 | 18:10 |
| morganfainberg | for submission | 18:10 |
| marekd | morganfainberg: yes | 18:10 |
| morganfainberg | oooh i need to figure out what i want to submit | 18:10 |
| dolphm | ayoung: they'll have to be merged or rejected quite a lot based on the fact that our schedule will be more constrained than the last summit | 18:10 |
| dstanek | morganfainberg: too late - new chain starts here https://review.openstack.org/#/c/79726/ | 18:10 |
| ayoung | marekd, I'm going to try to do something different this summit. For each session that I am involved with, I am going to write up a blog post, including the issues and my thoughts on the solution. Then, I'll post them long before the summit. If anyone joins the talk and hasn';t read them, I will publically berate them. | 18:10 |
| dolphm | morganfainberg: summit.openstack.org opened on friday | 18:11 |
| morganfainberg | dstanek, hehe ok | 18:11 |
| morganfainberg | dolphm, i think i have one that i want to propose | 18:11 |
| dolphm | morganfainberg: and will be open until april 20th | 18:11 |
| morganfainberg | dolphm, for keystone | 18:11 |
| ayoung | morganfainberg, get it up there | 18:11 |
| dolphm | marekd: ^ | 18:11 |
| marekd | ayoung: ok ;-) | 18:11 |
| morganfainberg | ayoung, yeah putting it up today | 18:11 |
| ayoung | nice | 18:11 |
| ayoung | morganfainberg, I suspect that, just like last time, we'll end up with "buckets" | 18:11 |
| ayoung | client, saml and ldap, token pipelien | 18:11 |
| ayoung | line | 18:11 |
| marekd | anyways, need to run, cheers! | 18:12 |
| *** marekd is now known as marekd|away | 18:12 | |
| ayoung | marekd, nice work | 18:12 |
| ayoung | I'lkl review, but plan on +2 | 18:12 |
| marekd|away | ayoung: thanks! | 18:12 |
| *** leseb has quit IRC | 18:13 | |
| ayoung | morganfainberg, I see you are in Janitor mode. Thanks for cleaning up the mess | 18:13 |
| *** leseb has joined #openstack-keystone | 18:13 | |
| *** daneyon has quit IRC | 18:13 | |
| *** daneyon has joined #openstack-keystone | 18:14 | |
| morganfainberg | ayoung, lol | 18:15 |
| morganfainberg | ayoung, yeah | 18:15 |
| *** leseb has quit IRC | 18:17 | |
| morganfainberg | ayoung, dolphm, http://summit.openstack.org/cfp/details/47 | 18:20 |
| morganfainberg | posted. | 18:20 |
| ayoung | Perf! | 18:20 |
| dolphm | dstanek: my only complaint with your series is that i'd prefer the addCleanup's to be as close as possible to whatever is being created that needs to be cleaned up... a couple of your changes increase that delta, but not significantly | 18:21 |
| dolphm | dstanek: just wanted to mention it | 18:21 |
| ayoung | morganfainberg, you only mentioned Ephemeral TOkens twice. Was that an oversight? | 18:21 |
| morganfainberg | ayoung, lol | 18:21 |
| morganfainberg | ayoung, i figured it warranted 2 mentioned, but didn't want to overload peopel | 18:21 |
| *** Fin1te has quit IRC | 18:22 | |
| dstanek | dolphm: there are definitely a few cases where i just grouped the cleanups together instead of adding a bunch of addCleanups | 18:24 |
| dolphm | ayoung: addressed http://summit.openstack.org/cfp/details/47 | 18:24 |
| bknudson | still not merged: https://review.openstack.org/#/c/79196/ | 18:24 |
| dolphm | ooh ^ someone review | 18:24 |
| morganfainberg | dolphm, looking now | 18:25 |
| ayoung | dolphm, looking | 18:25 |
| morganfainberg | really? | 18:25 |
| morganfainberg | missed that arg | 18:25 |
| morganfainberg | doh! | 18:25 |
| dolphm | morganfainberg: that's the second time it's been broken in like a year :P | 18:26 |
| morganfainberg | +2/+A | 18:26 |
| dolphm | morganfainberg: i think bcwaldon fixed it last | 18:26 |
| morganfainberg | well now we test! | 18:26 |
| morganfainberg | bknudson, _++++++ | 18:26 |
| bknudson | it got caught in the middle of oslo.db sync and the other migration refactoring for revoked tokens. | 18:27 |
| morganfainberg | bknudson, ahh | 18:27 |
| bknudson | and the lack of tests didn't help | 18:27 |
| ayoung | that was a nightmare to keep straight | 18:27 |
| morganfainberg | yeah | 18:27 |
| dolphm | bknudson: +++ | 18:27 |
| ayoung | things kept changing out from underneath me | 18:27 |
| bknudson | maybe we should put a call to keystone-manage db_version in devstack | 18:27 |
| ayoung | bknudson, sql livetests | 18:27 |
| morganfainberg | bknudson, oh that would be cool | 18:27 |
| *** saju_m has quit IRC | 18:30 | |
| dolphm | stevemar: dstanek responded to your -1 in https://review.openstack.org/#/c/79728/1/keystone/tests/test_backend.py | 18:30 |
| stevemar | dolphm, ah so he did | 18:31 |
| ayoung | bknudson, https://bugs.launchpad.net/openstack-ci/+bug/118209 | 18:32 |
| ayoung | bah | 18:32 |
| ayoung | wrong bug.... | 18:32 |
| bknudson | ayoung: it's too bad about gutsy missing icons! | 18:32 |
| bknudson | I'll get right on it. | 18:32 |
| ayoung | https://bugs.launchpad.net/openstack-ci/+bug/1182092 | 18:33 |
| bknudson | ayoung: ah, great. | 18:33 |
| ayoung | bknudson, yeah...tooo bad about launchpad not keeping bugs for separate projects apart, too. | 18:33 |
| dstanek | there seem to be a good amount of bugs related to data validation :-) do we have a strategy layed out or a prototype of adding a validation layer? | 18:33 |
| bknudson | ayoung: I never know what the infra team wants to include tests for... | 18:33 |
| bknudson | it seems like lately they've been pushing back on adding new tests. | 18:33 |
| bknudson | internally here we're working on publishing CI results for keystone with db2. | 18:34 |
| ayoung | bknudson, I was hoping to do something along lines of mixing an LDAP and SQL backends for a single test, using henrynash's code, but since that is not going to make Icehouse | 18:34 |
| dolphm | dstanek: ++ only a general preference for using jsonschema | 18:34 |
| stevemar | dstanek, thanks for the response! | 18:34 |
| bknudson | ayoung: we can mix ldap and sql... just not multi-ldap? | 18:34 |
| ayoung | I think what we can do is to add a migration test and sql based unit test with a real DB next to the mysql and postgres runs | 18:34 |
| morganfainberg | dstanek, +2 the whole chain. once Jenkins weighs in, +A no question | 18:34 |
| bknudson | or is it not worth it to have ldap identity and sql assignment. | 18:35 |
| dstanek | dolphm: ok, i may POC a decorator based approach to that we have something to start looking at | 18:35 |
| ayoung | so long as the tests use a different DB instance (keystone_tests versus keystone) | 18:35 |
| morganfainberg | dstanek, once those are in i'll restructure my cleanup for the config files. | 18:35 |
| morganfainberg | and we can land config file stuff in J1 or in I (doesn't matter in either case) | 18:35 |
| dstanek | morganfainberg: nice, i like removing the usage of sample | 18:36 |
| morganfainberg | heck yeah | 18:36 |
| dolphm | dstanek: my first question is how you're defining schemas for creates vs updates on a single resource? | 18:36 |
| bknudson | ayoung: ok. I'll try to get the team here to do the same for db2. | 18:36 |
| ayoung | bknudson, that makes sense. We might need to split the livetest files into a DB specific file per | 18:36 |
| ayoung | _livetests_sql_db2.py and so on | 18:37 |
| bknudson | ayoung: I hope we don't have anything different depending on the db in use. | 18:37 |
| dstanek | dolphm: i'll answer you with a unit test on a demo :-) I have been thinking about this and have ideas, but i don't yet know if they'll work | 18:37 |
| bknudson | ayoung: I assume it's just the connection string? | 18:37 |
| ayoung | bknudson, we'll use a differenc config file, that should be it | 18:38 |
| bknudson | ayoung: that might be better to do with an env var or something. | 18:38 |
| morganfainberg | bknudson, so you'd do the db2 stuff like nova's minesweeper/turbo-hipster/whatever? | 18:38 |
| ayoung | well, you want to be able to do | 18:38 |
| dstanek | when are we dropping XML support? there is at least on bug related to XML validation | 18:38 |
| bknudson | morganfainberg: yes... there's already some db2 ci in place, for sqlalchemy. | 18:38 |
| ayoung | testr keystone.tests._livetest_sql | 18:38 |
| ayoung | or some way to trigger the test | 18:38 |
| ayoung | you could do it as an external env var, but, yuck | 18:38 |
| bknudson | ayoung: we should look at how tempest picks which tests to run. | 18:39 |
| bknudson | they have smoke tests and other types. | 18:39 |
| ayoung | bknudson, also, might want to get the config file from somewhere out of the tree for a live db | 18:39 |
| morganfainberg | bknudson, woo! | 18:39 |
| ayoung | bknudson, I think we need a tox target | 18:39 |
| bknudson | ayoung: getting the config file from outside the tree makes sense... will probably need it for the test infrastructure to provide it to keystone anyways. | 18:39 |
| ayoung | bknudson, maybe | 18:40 |
| ayoung | not sure what they do now, but I assume they just use devstack defaults | 18:40 |
| *** petertoft has quit IRC | 18:41 | |
| *** andreaf has quit IRC | 18:45 | |
| *** bobt has joined #openstack-keystone | 18:53 | |
| *** bobt has quit IRC | 19:06 | |
| openstackgerrit | Brant Knudson proposed a change to openstack/keystone: Update sample config https://review.openstack.org/78024 | 19:13 |
| openstackgerrit | Brant Knudson proposed a change to openstack/keystone: Use oslo db.sqlalchemy.session.EngineFacade.from_config https://review.openstack.org/78459 | 19:13 |
| openstackgerrit | Brant Knudson proposed a change to openstack/keystone: Sync db, db.sqlalchemy from oslo-incubator 0a3436f https://review.openstack.org/78429 | 19:13 |
| bknudson | https://review.openstack.org/78429 fixes the nasty WARNING message from oslo-incubator about the database mode. | 19:14 |
| *** raildo has joined #openstack-keystone | 19:14 | |
| bknudson | that was printed when the server starts and during db_sync. | 19:14 |
| *** zhiyan is now known as zhiyan_ | 19:20 | |
| *** daneyon has joined #openstack-keystone | 19:31 | |
| raildo | hello, I created this blueprint https://blueprints.launchpad.net/keystone/+spec/filter-user-by-project-v3 if someone wants to discuss, approve, or something like that =] | 19:32 |
| *** topol has joined #openstack-keystone | 19:47 | |
| dolphm | last patch for an rc1 blocker needs reviews (this is just new tests) https://review.openstack.org/#/c/77375/ | 20:07 |
| bknudson | dolphm: do you think https://bugs.launchpad.net/oslo/+bug/1271706 is rc1 blocker? | 20:09 |
| morganfainberg | bknudson, i would love if we could sanely smash that bug | 20:10 |
| morganfainberg | bknudson, if it is something easy (wont break things) it would be great to have fixed for I | 20:10 |
| dolphm | bknudson: just targeted it as such | 20:10 |
| morganfainberg | dolphm, ++ | 20:10 |
| bknudson | thanks... I've been getting complaints about it | 20:11 |
| morganfainberg | dolphm, looks like dstanek's changes are positive. should be good to go soon | 20:11 |
| morganfainberg | dolphm, i'll press +A on each of them once they are good if someone else doesn't beat me to it | 20:11 |
| bknudson | in reviews now do we need to -1 if somebody leaves a string referenced in their test class? | 20:16 |
| morganfainberg | bknudson, explain? | 20:17 |
| bknudson | morganfainberg: aren't those the changes that dstanek is making? | 20:18 |
| morganfainberg | oh failing to cleanup? | 20:18 |
| bknudson | morganfainberg: yes | 20:18 |
| morganfainberg | ideally yes. we should point them to how to clean it up | 20:18 |
| morganfainberg | esp. if we want to keep test bloat to a minimum | 20:18 |
| morganfainberg | i think we'll have some extra work to do to get things in-line since things merged since dstanek wrote those patches | 20:19 |
| morganfainberg | but i think we can document / point people in that direction | 20:19 |
| morganfainberg | or do a quick "here fixed it for ya, next time do this" | 20:19 |
| * dolphm p.s. i'll be afk thursday & friday this week | 20:20 | |
| morganfainberg | dolphm, enjoy being afk (if possible) | 20:20 |
| dolphm | morganfainberg: more enjoyment will be had if https://review.openstack.org/#/c/79284/ and https://review.openstack.org/#/c/77375/ are approved first ;) | 20:21 |
| morganfainberg | i think we can get those through today | 20:21 |
| * morganfainberg has them in windows to look at as soon as i get food | 20:21 | |
| dolphm | anxious to see subsequent patchsets on https://review.openstack.org/#/c/76476/ and https://review.openstack.org/#/c/78521/ too | 20:23 |
| bknudson | dolphm: cjellick said he'd have a new revision of https://review.openstack.org/#/c/78521/ up yesterday... | 20:23 |
| morganfainberg | ick, we allow referral chasing... well don't disable it | 20:23 |
| morganfainberg | ick | 20:23 |
| bknudson | morganfainberg: I think referral chasing in python is mostly broken, since it doesn't pass on the credentials | 20:24 |
| morganfainberg | bknudson, yep | 20:24 |
| morganfainberg | bknudson, ran across that when i worked at the former large social media company that facebook superseded. | 20:24 |
| morganfainberg | referral chasing was just... broken | 20:25 |
| bknudson | morganfainberg: it'll probably make a comeback once too many adults get on facebook. | 20:25 |
| morganfainberg | no thanks i'll pass on seeing the resurgence if myspace. | 20:26 |
| bknudson | morganfainberg: too many unicorns? | 20:26 |
| morganfainberg | here let me bling that up for you | 20:26 |
| morganfainberg | bknudson, in short, ues | 20:26 |
| morganfainberg | yes* | 20:26 |
| * dolphm relevant https://www.youtube.com/watch?v=7mFJdOsjJ0k | 20:28 | |
| morganfainberg | oh no.. risky click of the day? :P | 20:28 |
| morganfainberg | LOLLLL | 20:29 |
| morganfainberg | s/LLL/ | 20:29 |
| morganfainberg | i kinda like jamielennox|away's positional decorator | 20:29 |
| morganfainberg | it makes python feel less "oops i tossed arguments in and got something out" syntax wise | 20:30 |
| bknudson | morganfainberg: I wanted to look at that... I'd seen an interesting stackoverflow on this. | 20:31 |
| dolphm | morganfainberg: link? | 20:31 |
| bknudson | http://legacy.python.org/dev/peps/pep-3102/ | 20:33 |
| bknudson | so you could do "def compare(a, b, *ignore, key=None):" and then "if ignore: raise TypeError" already | 20:34 |
| bknudson | I don't know if "def compare(a, b, *, key=None):" works in python3 already? | 20:34 |
| dolphm | nice | 20:35 |
| ayoung | Funny thing is that one of the features that Java is finally getting in Java 8 is that it will no longer throw out the parameter names in the reflection data about a function call. | 20:35 |
| dstanek | bknudson: yes it does in 33 | 20:35 |
| ayoung | So you would finally be able to convert from a dictionary to the arguments of a method | 20:35 |
| ayoung | its only taken 20 years | 20:35 |
| dstanek | bknudson: jamielennox|away has a decorator under review for similar functionality; to be used on methods that make up our external API | 20:36 |
| dstanek | ayoung: what do you mean? | 20:37 |
| bknudson | unfortunately you have to supply the number of positionals... | 20:37 |
| bknudson | could indicate that with a *ignore or something in the arguments instead. | 20:37 |
| dstanek | bknudson: not anymore, i gave him a snippet that takes care of that | 20:37 |
| dolphm | dstanek: it seems like you'd end up using , *, in every single method signature where you have both args and kwargs ... is that true? | 20:37 |
| morganfainberg | dolphm, here is the review jamie proposed https://review.openstack.org/#/c/77026/ | 20:37 |
| ayoung | dstanek, so before you could get an object and enumerate its methods. and take a method and enumerate the types it took in a parameter list. But you could not actually get the parameter names | 20:37 |
| morganfainberg | bknudson, ^ | 20:38 |
| ayoung | they were tossed out at compile time | 20:38 |
| ayoung | so if you had a function that takes 3 strings, you don;t know which is which unless you do it positionally | 20:38 |
| dstanek | dolphm: only the ones where you wanted to control how it's being called and you don't have control of the calling code | 20:38 |
| ayoung | and considering that creating a db connection is usually String url, String userid, String password.... | 20:38 |
| dolphm | dstanek: "don't have control of the calling code" == "all public methods" ? | 20:39 |
| dstanek | ayoung: you've been able to get parameter names of a method using inspect for a long time | 20:39 |
| ayoung | its funny, dstanek because I consider this the killer feature, but they've been holding up the release due to Lamdas | 20:39 |
| ayoung | dstanek, in Java | 20:39 |
| dstanek | dolphm: yes, whatever is advertised as a public method (and you want control) | 20:40 |
| dstanek | dolphm: i think that in some ways the need for this is driven from a fragmented model | 20:40 |
| *** nkinder_ has quit IRC | 20:41 | |
| bknudson | let's just drop python2 support | 20:42 |
| morganfainberg | bknudson, i'd love to see py2 go away :) | 20:42 |
| morganfainberg | bknudson, i think we're getting closer to py3 being the default install around... but still probably a few OS releases away | 20:44 |
| ayoung | morganfainberg, we need to drive a fix for eventlet | 20:46 |
| morganfainberg | ayoung, can we ditch eventlet instead :P | 20:46 |
| ayoung | HA! | 20:46 |
| ayoung | since we're dreaming, I'd like....hmmmm..... | 20:46 |
| morganfainberg | ayoung, actually i think it's a different bug now *goes and looks* | 20:46 |
| ayoung | wasn';t last week | 20:46 |
| morganfainberg | ayoung, can't have a pony, i already requisitioned one | 20:46 |
| bknudson | I'm not sure what all eventlet does for keystone... isn't it blocked on the db all the time anyways? | 20:47 |
| morganfainberg | bknudson, it makes a WSGI stand-alone framework? | 20:47 |
| morganfainberg | or a wrapper to call that wsgi framework | 20:47 |
| bknudson | morganfainberg: there must be other ways to do that... plus ayoung wants to run in apache anyways. | 20:48 |
| ayoung | eventlet does nothing for us AFAICT | 20:48 |
| morganfainberg | bknudson, i keep trying to work on making devstack play nice w/ keystone on port 80 shared w/ horizon | 20:48 |
| morganfainberg | wow. it's kindof a pain the way the config files are built | 20:49 |
| morganfainberg | i kinda wish it used a CMS instead of hacky bash-isms | 20:49 |
| ayoung | I wish it was in Python | 20:50 |
| openstackgerrit | Brant Knudson proposed a change to openstack/keystone: Remove unnecessary oauth1.Manager constructions https://review.openstack.org/79213 | 20:51 |
| morganfainberg | ayoung, python, puppet, chef, cfengine, whatever | 20:51 |
| ayoung | Its has outgrown what you should do in Bash | 20:51 |
| ayoung | no, Python | 20:51 |
| ayoung | one language is easier to understand than twpo | 20:51 |
| ayoung | two | 20:51 |
| morganfainberg | ayoung, devstack likely should be puppet based tbh. it's what infra uses.. | 20:51 |
| morganfainberg | combine efforts | 20:51 |
| morganfainberg | could use saltstack | 20:51 |
| morganfainberg | that's python | 20:51 |
| bknudson | I expect that sometime devstack will go away for chef or puppet or something. | 20:52 |
| morganfainberg | bknudson, likely | 20:52 |
| morganfainberg | or devstack will just be a pretty wrapper for puppet/chef | 20:52 |
| morganfainberg | run this script and it does the extra magic | 20:52 |
| morganfainberg | actually i wonder how hard that would be to convert to. | 20:52 |
| dolphm | morganfainberg: that script: https://github.com/dolph/keystone-deploy | 20:52 |
| morganfainberg | dolphm, ooh | 20:53 |
| haneef__ | ayoung: is the following bug or something unqiue to ldaps.? In the code we don't set cacertfile option if the protocol is ldaps. We do only for start_tls. Won't ldaps protocol require cert? | 20:53 |
| dstanek | these are still a handful of requirements in requirements.txt that don't play nice on py3 | 20:53 |
| dstanek | s/these/their/ | 20:54 |
| ayoung | haneef__, I have no idea. In order to remember I would have to forget all of the python-keystoneclient stuff in my brain right now | 20:54 |
| morganfainberg | dstanek, yeah | 20:54 |
| ayoung | I'll let you tell me | 20:54 |
| dstanek | i wish there was a way to list requirements by Python version | 20:54 |
| dolphm | morganfainberg: my goal there is to have the recommended production configuration for 50% of deployments in the master branch, and then illustrate a few popular alternatives (backing to LDAP, federated, etc) in branches | 20:54 |
| morganfainberg | dolphm, hmm. should i submit a session for devstack saying "maybe we should move away from bash" for Juno summit? | 20:55 |
| morganfainberg | dolphm, awesome, let me look at it, and if i can toss some pull requests you way i will | 20:55 |
| dolphm | morganfainberg: it's been brought up before, and hasn't made any traction | 20:55 |
| morganfainberg | dolphm, /me thinks it'll just be ignored as a summit session | 20:55 |
| morganfainberg | perhaps the only way to do it is propose a devstack 2.0 that is using different tech. | 20:56 |
| morganfainberg | do all the work at once. | 20:56 |
| dolphm | there was a project at some point called pystack or stack.py or something that was basically a port of early devstack to python | 20:56 |
| morganfainberg | icky | 20:56 |
| dstanek | bknudson: are your tests here https://review.openstack.org/#/c/77375/13/keystone/tests/test_backend_kvs.py just to make sure the parent's impl is not implemented? | 20:56 |
| morganfainberg | dolphm, the hard part is getting enough adoption to make it the gate target | 20:57 |
| bknudson | dstanek: yes, just shows how the test works today... if somebody implements it in future it will be easy to find. | 20:57 |
| morganfainberg | without that it'll just wither and die | 20:57 |
| *** raildo has quit IRC | 20:57 | |
| dolphm | morganfainberg: ++ i'd start as 3rd party CI and work from there | 20:57 |
| *** leseb has joined #openstack-keystone | 20:58 | |
| morganfainberg | dolphm, interesting idea | 20:58 |
| morganfainberg | dolphm, if i had cash to spare i'd donate some resources for that | 20:58 |
| morganfainberg | alas, i'm not paid that much :P | 20:58 |
| dstanek | morganfainberg: i started a port to saltstack a while back as a way to learn more about devstack :-) | 20:58 |
| morganfainberg | dstanek, yeah | 20:58 |
| morganfainberg | dstanek, i worked on something similar not too long ago | 20:59 |
| morganfainberg | didn't get too far | 20:59 |
| dolphm | morganfainberg: if we have code to run, i can probably wrangle some VMs | 20:59 |
| morganfainberg | dolphm, hm, maybe i'll make it a weekend project. | 20:59 |
| morganfainberg | dolphm, we can start by having keystone use it as external CI :) | 20:59 |
| dolphm | morganfainberg: ++ | 21:00 |
| morganfainberg | i think mirroring the devstack-full gate box would be the initial target | 21:00 |
| *** marcoemorais has quit IRC | 21:01 | |
| dstanek | i just added the second +2 to https://review.openstack.org/#/c/77375/ - any reason not to approve it at this time? | 21:01 |
| *** nkinder has joined #openstack-keystone | 21:01 | |
| morganfainberg | dstanek, i say +A it | 21:01 |
| *** marcoemorais has joined #openstack-keystone | 21:02 | |
| dolphm | dstanek: go for it! | 21:02 |
| morganfainberg | dstanek, i approved the patches in your chain that passed check | 21:02 |
| morganfainberg | waiting for a recheck or two | 21:02 |
| morganfainberg | 2nd and last ones failed for known bug reasons | 21:02 |
| morganfainberg | as soon as they pass everything should hit gate | 21:02 |
| dstanek | morganfainberg: great, thanks | 21:03 |
| *** marcoemorais has quit IRC | 21:03 | |
| *** marcoemorais has joined #openstack-keystone | 21:03 | |
| dstanek | ok, off to look at https://review.openstack.org/#/c/79284 again! | 21:03 |
| *** gyee has quit IRC | 21:05 | |
| *** marcoemorais has quit IRC | 21:08 | |
| *** marcoemorais has joined #openstack-keystone | 21:09 | |
| *** daneyon has quit IRC | 21:10 | |
| *** daneyon has joined #openstack-keystone | 21:11 | |
| dstanek | dolphm, stevemar: the commit message on https://review.openstack.org/#/c/79284 is incorrect now, right? We're not discarding groups anymore, we're raising a server error | 21:11 |
| stevemar | dstanek, yeshhh | 21:12 |
| dstanek | stevemar: k, that's what i thought. i can submit an update to it. | 21:14 |
| stevemar | dstanek, cool | 21:21 |
| *** stevemar has quit IRC | 21:22 | |
| openstackgerrit | Steve Martinelli proposed a change to openstack/python-keystoneclient: Add request/access token and consumer support for keystoneclient https://review.openstack.org/60820 | 21:23 |
| *** stevemar has joined #openstack-keystone | 21:23 | |
| stevemar | morganfainberg, i addressed your concerns here: https://review.openstack.org/#/c/60820/18 | 21:23 |
| stevemar | dolphm, ayoung ^ | 21:23 |
| openstackgerrit | Steve Martinelli proposed a change to openstack/python-keystoneclient: Authenticate via oauth https://review.openstack.org/77977 | 21:24 |
| openstackgerrit | David Stanek proposed a change to openstack/keystone: Enforce groups presence for federated authn https://review.openstack.org/79284 | 21:26 |
| dstanek | dolphm, stevemar: i just updated the commit message ^ | 21:27 |
| stevemar | dstanek, thanks boss | 21:27 |
| *** marcoemorais has quit IRC | 21:30 | |
| *** marcoemorais has joined #openstack-keystone | 21:30 | |
| *** stevemar has quit IRC | 21:43 | |
| *** thedodd has quit IRC | 21:44 | |
| *** leseb has quit IRC | 21:46 | |
| *** leseb has joined #openstack-keystone | 21:47 | |
| *** leseb has quit IRC | 21:51 | |
| *** topol has quit IRC | 21:52 | |
| *** devlaps1 has joined #openstack-keystone | 22:02 | |
| *** devlaps has quit IRC | 22:02 | |
| *** daneyon has quit IRC | 22:29 | |
| *** harlowja has quit IRC | 22:30 | |
| *** harlowja has joined #openstack-keystone | 22:31 | |
| *** nkinder has quit IRC | 22:40 | |
| *** jamielennox|away is now known as jamielennox | 22:40 | |
| *** Daviey has quit IRC | 22:46 | |
| *** nkinder has joined #openstack-keystone | 22:47 | |
| *** Daviey has joined #openstack-keystone | 22:47 | |
| *** leseb has joined #openstack-keystone | 22:57 | |
| *** leseb has quit IRC | 23:01 | |
| *** dims_ has quit IRC | 23:02 | |
| *** gordc has quit IRC | 23:05 | |
| *** nkinder has quit IRC | 23:13 | |
| *** dims_ has joined #openstack-keystone | 23:16 | |
| openstackgerrit | Jamie Lennox proposed a change to openstack/python-keystoneclient: Add a positional decorator https://review.openstack.org/77026 | 23:17 |
| openstackgerrit | Jamie Lennox proposed a change to openstack/python-keystoneclient: Start using positional decorator https://review.openstack.org/77055 | 23:18 |
| *** henrynash has joined #openstack-keystone | 23:23 | |
| *** gyee has joined #openstack-keystone | 23:24 | |
| *** wchrisj has quit IRC | 23:26 | |
| *** nkinder has joined #openstack-keystone | 23:28 | |
| *** david_lyle_ has joined #openstack-keystone | 23:39 | |
| jamielennox | how's zuul doing at the moment? i've reverified this review now 6 times and it fails: https://review.openstack.org/#/c/79474/ | 23:39 |
| jamielennox | (no, it's not my fault) | 23:39 |
| *** david-lyle has quit IRC | 23:41 | |
| *** wchrisj has joined #openstack-keystone | 23:48 | |
| *** gokrokve_ has quit IRC | 23:54 | |
| *** gokrokve has joined #openstack-keystone | 23:55 | |
| bknudson | jamielennox: what do you think about having the positional decorator use something more like py3 / pep 3102 -- http://legacy.python.org/dev/peps/pep-3102/ | 23:56 |
| jamielennox | bknudson: let me check | 23:57 |
| bknudson | jamielennox: here they say you can simulate it in py27 using "def compare(a, b, *ignore, key=None):" | 23:57 |
| bknudson | we'd still need a decorator... it could check if the *ignore argument is empty | 23:58 |
| bknudson | does the decorator know the names of the arguments? | 23:58 |
| *** gokrokve has quit IRC | 23:59 | |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!