Tuesday, 2014-03-04

dolphmbknudson: fixed nits in https://review.openstack.org/#/c/56243/00:01
dolphmbknudson: thanks!00:11
*** leseb has quit IRC00:19
*** rwsu has quit IRC00:27
*** rwsu has joined #openstack-keystone00:30
*** andreaf has quit IRC00:31
*** achampion has joined #openstack-keystone00:42
*** richm has quit IRC00:52
*** dims has joined #openstack-keystone00:54
bknudsonno more v2 deprecated messages!01:02
bknudsonexcept for the first one.01:02
jamielennoxbknudson: from where?01:03
morganfainbergbknudson, yay01:03
bknudsonin the keystone server log01:03
morganfainbergdolphm, are we punting on the per-domain backend stuff till J?01:03
bknudson"DEBUG stevedore.extension " -- should probably get rid of those. At least it only happens once.01:03
morganfainbergi noticed it was moved to "next"?01:04
dolphmmorganfainberg: i at least don't want to block I on it with as much conversation/disagreement as it's producing. if we settle on a single solution *very* quickly i'd be happy to see something in icehouse01:04
bknudsondid we deprecate the v2 client api, too?01:04
morganfainbergdolphm, ++ sounds good01:05
dolphmbknudson: the lib?01:05
bknudsondolphm: yes, in python-keystoneclient01:05
dolphmbknudson: no01:05
bknudsondidn't remember seeing it... any reason to not deprecate it?01:05
jamielennoxbknudson: no we don't deprecate the client01:05
jamielennoxthat requires a major semver01:05
jamielennoxbut i'm not sure we need to be in a rush on the client side01:06
bknudsonit requires a 1.0 client to deprecate something? I thought that was when it was removed.01:06
dolphmthe CLI help includes a pending deprecation message01:06
jamielennoxbknudson: i would like to do proper semver for the clients - i might be aiming to high01:06
dolphmi'd like to upgrade that to a proper 'deprecated' warning soon01:07
dolphmi want to make sure the UX in python-openstackclient is up to par (the hardcoded /v2.0/ catalog issue is the only pain point there afaict)01:07
dolphmbknudson: we need to land this in tempest too https://review.openstack.org/#/c/76949/01:08
dolphmwaiting on a recheck :(01:08
bknudsondolphm: it's the crappy xml serializer01:09
bknudsonwe should deprecate that.01:09
dolphmbknudson: ours? we did01:09
bknudsonjamielennox: we don't do proper semver for the clients?01:10
dolphmbknudson: https://review.openstack.org/#/c/76301/01:10
jamielennoxbknudson: i think clients break compatability all over the place01:10
bknudsondolphm: can tempest stop testing it now?01:10
dolphmbknudson: i think it should be tested as long as it's supported :-/01:11
dolphmbknudson: otherwise that commit would have contained a ton of deletes on our side :D01:11
bknudsondolphm: the v3 catalog response is broken -- https://review.openstack.org/#/c/77375/9/keystone/tests/test_v3_auth.py01:12
bknudsonline 203601:12
morganfainbergdolphm, DELETE ALL THE THINGS!01:12
bknudsonI mean the catalog in the v3 auth response.01:13
dolphmbknudson: oh wow01:13
dolphmbknudson: i hadn't gotten that far in the sequence01:13
jamielennoxbknudson: lol - in a shoot me kind of way01:16
*** rwsu has quit IRC01:17
dolphmmorganfainberg: jamielennox: bknudson: i manually deleted keystone.openstack.common.crypto and forgot to remove the entry from openstack-common.conf; latest patchset fixes that https://review.openstack.org/#/c/77701/01:22
morganfainbergdolphm, ah01:22
morganfainbergok01:22
bknudsonthat caused the docs to fail?01:22
dolphmbknudson: sort of but not ours-- we're building docs for keystone.openstack.common which failed as a result01:27
bknudsondolphm: docs built fine for me locally with that change.01:27
*** stevemar has joined #openstack-keystone01:28
*** ChanServ sets mode: +v stevemar01:28
dolphmbknudson: the patchset previous to that failed (where i deleted the pycrypto requirement but didn't delete keystone.common.common.crypto which tried to import it)01:28
dolphmbknudson: with common.crypto gone, the docs did build, but i obviously didn't make the change properly01:28
*** leseb has joined #openstack-keystone01:30
*** ayoung_afk is now known as ayoung01:30
morganfainbergdolphm, what is the maximum size user_id derived section we are willing to accept?01:34
*** leseb has quit IRC01:34
morganfainbergdolphm, mulling over some alternatives01:35
jamielennoxbknudson: we talked a while ago about replacing auth_host, auth_port etc in auth_token middleware with just one uri01:36
jamielennoxbknudson: i posted https://review.openstack.org/#/c/77491/ with the config 'admin_uri'01:36
jamielennoxis admin_uri sufficiently distinguished from auth_uri?01:36
bknudsonjamielennox: that was a while ago!01:37
jamielennoxbknudson: yea, something was wrong back then that it couldn't be done very easily01:37
jamielennoxi tried it again and it worked out well01:37
jamielennoxor at least fairly easily01:37
bknudsonjamielennox: maybe we could name it based on how it's used...01:39
jamielennoxbknudson: you can review when you have time but does admin_uri work?01:39
bknudsonhow it's used by auth_token01:39
bknudsondoesn't seem like we should say admin endpoint... v3 isn't really an admin endpoint?01:40
jamielennoxso it appears that we've always advocated that auth_port should be 35357 so we are talking to the admin, rather than auth port01:40
jamielennoxbknudson: right - but it's weird for v301:40
bknudsonIt's called 'request_uri' in the code.01:41
jamielennoxyea, but i remember doing that purely because i couldn't come up with anything better01:41
bknudsoncould call it 'identity_uri'01:42
jamielennoxyea, that's ok - i think just uri is too short01:42
jamielennoxcan you validate a UUID token against the public endpoint in v2?01:42
bknudsonpeople might think 'uri geller' spoon bending01:42
jamielennoxdo we care about people using the admin port for this any more?01:43
jamielennoxi'm sure it works to put the public endpoint in v201:44
bknudsonauth_token needs the v2 admin endpoint to get the certs.01:44
jamielennoxah, the certs01:45
bknudsondid that finally get ported to v3?01:45
jamielennoxyea01:45
jamielennoxnot supported by auth_token yet though01:45
jamielennoxso we are stuck with having the new 'identity_uri' conf option pointing to the admin port01:47
bknudsonjamielennox: I'm leaning towards identity_uri , and document that you set it to the v2 admin endpoint ?01:47
bknudsonin the hopes of eventually changing it to unversioned endpoint.01:47
jamielennoxbknudson: yea - i would like to doc that it should be the public01:47
jamielennoxah, you don't need the v2 endpoint01:47
jamielennoxauth_token has always worked without the endpoint01:48
bknudsonprovide an example in the doc01:48
jamielennox(versioned endpoint)01:48
bknudsonor a suggested value01:48
bknudsonI guess you can't set the default because that would override the deprecated values...01:49
bknudsonjamielennox: how about setting the deprecated options to default=None and have admin_uri have a default?01:49
jamielennoxbknudson: then i would never be able to tell if the identity_api was unset01:50
bknudsonyou'd use the deprecated options only if they were set.01:51
bknudsononly if any was set, or something.01:51
jamielennoxoh, reverse it01:51
jamielennoxhmm, i see the point in documenting a default but it does make it kind of the wrong way round01:52
jamielennoxyou'd be saying if auth_host is set it would override identity_uri01:52
bknudsonjamielennox: right, if it was set explicitly01:52
jamielennoxin the case of a misconfigure though where someone sets auth_host, auth_port and identity_uri i want to take identity_uri and ignore the rest01:53
jamielennoxalso because of the defaults of eg auth_schema, if you set auth_host and nothing else it will build a correct url01:53
bknudsonjamielennox: ok, was just thinking it would be nice to be able to have a default... I guess conf doesn't let us check if it was set.01:54
jamielennoxbknudson: yea, i agree but we can't tell between default and set01:54
*** amcrn has quit IRC01:55
jamielennoxi'll put an eg in the doc and that should suffice for now01:55
bknudsonjamielennox: an egg in the doc works for me.01:55
*** devlaps has quit IRC02:01
ayoungbknudson, whenever I build docs I get a slew of errors like   File "/opt/stack/keystone/.tox/venv/lib/python2.7/site-packages/sphinx/ext/autodoc.py", line 321, in import_object02:11
ayoung    __import__(self.modname)02:11
ayoungImportError: No module named rpc.zmq_receive02:11
ayoungany advice?02:11
ayoung WARNING: autodoc can't import/find module 'keystone.openstack.common.notifier', it reported error: "No module named notifier", please check your spelling and sys.path02:11
ayoungmorganfainberg, I can't help but think that there is some clear solution right under out noses with the id mapping thing.02:12
morganfainbergayoung, i'm looking into some alternatives now02:12
ayoungcool02:13
morganfainbergayoung, i _might_ have something usable.02:13
morganfainbergayoung, but trying it out first02:13
*** browne has quit IRC02:13
ayoungmorganfainberg, dumb idea, just to generate a smart one...what if we used PKI02:15
morganfainbergayoung, not far off the idea i had02:15
ayoungso the user id is .... encrypted with the private key02:15
ayounghashed or something02:15
ayoungnah...02:15
ayoungit needs to be reversable02:16
morganfainbergayoung, that is what i'm aiming for02:16
ayoungbut...yeah, a key encrypted should be short, no?02:16
morganfainbergayoung, except the padding of encrypt will likely overflow our small dataset (64bytes)02:16
ayoungPKI is expensive, though02:16
ayoungwhat if it was not asym02:17
morganfainbergayoung, i'm looking into some b64 encode magic see if i can do some stuff with mangling up the format of the uuids etc.02:17
morganfainbergayoung, right now i can consistently build 31byte user_names and a domain uuid in 64bytes urlsafe reversible02:17
ayoungmorganfainberg, I owe you beer just for getting that far02:18
morganfainbergtrying to see if i can increase the user_name element size some way02:18
ayoungchop one byte off the domain id02:18
morganfainbergayoung, hehe i'm only using 16bytes for the domain id02:18
morganfainbergayoung, uuid.uuid4().bytes02:18
ayoungah02:19
morganfainbergwell, 17, an explicit \0 for the delimiter between the two elements02:19
ayoungleave that off02:19
morganfainberghow do you split between then?02:19
ayoungfixed length02:19
morganfainberghm..02:19
ayoungjust  a thought...if every byte counrs02:19
morganfainbergwas having an issue with slicing getting confused w/ bytes02:19
ayoungcounts02:19
morganfainberglet me try it again02:20
morganfainbergalso if it's a uuid user_id i compress it down to 16 bytes as well02:20
ayoungacha02:21
ayoungWe won't be mixing old UUIDs and new ones together this way, though02:21
morganfainbergayoung, but if we can keep the string smaller, why not?02:22
ayoungah02:22
ayoungunles we go by length02:22
morganfainbergand i can eliminate the \002:22
morganfainbergi was typoing before02:22
ayoung32byte = straight UUID.  64byte = new style02:23
ayoungExisting LDAP is going to be problematic, have to be grandfathered in02:23
morganfainbergayoung, http://paste.openstack.org/show/71982/02:25
morganfainbergayoung, simple test code, but something like that02:25
*** lbragstad has joined #openstack-keystone02:25
morganfainbergwe could remove the extra magic uuid thing to shorten the user_id02:26
morganfainbergayoung, all user_ids would be grandfathered in02:28
morganfainbergwe can detect if it's a "new-style" id02:28
ayoungmorganfainberg, if the userid < 64 its a grandfathered value?02:28
morganfainbergnah, if not b64encode02:28
ayoungUUID4.hex won't accidentally show up as b64?02:29
morganfainbergdon't think so.02:29
dolphmjamielennox: bknudson: morganfainberg: jenkins-shaped +1 on https://review.openstack.org/#/c/77701/02:29
morganfainbergayoung, doh it will02:29
dolphmalso there's a storm going through here and our power keeps going out02:29
*** gokrokve has quit IRC02:30
morganfainbergdolphm, LGTM +202:30
lbragstadnice, I was +1 on that before02:30
jamielennoxdolphm: i guess it's only fitting i do the +A02:31
jamielennoxsniff, i hope we see you again KDS02:31
dolphmjamielennox: i'll +A the delete all of keystone in the branch then :P02:31
ayoungdolphm, thanks for keeping up with the revoke patch.  I was keeping the Fake backend around as an escape hatch, but we don't need it anymore.  Using the events removed most of the dependencies from the revoke code, leaving only the token provider.  I made it optional there, and now if you don't enable the revoke backend, there should be no trace of it....02:32
ayoungwhich leads to the question of whether we should disable it or enable it by default02:32
ayoungIf disabling it makes people feel more comfortable....and increases the chance of it going it, I am more than willing to do so02:32
morganfainbergayoung, hmm. trying to figure out how to differentiate uuid from "new" id02:32
ayoungmorganfainberg, length02:32
jamielennoxdolphm: i want https://review.openstack.org/#/c/77525/ before any client release - just in case you were getting ideas02:33
morganfainbergayoung, i guess we could pad everything up to 64 always.02:33
ayoungjamielennox, ++ on that02:33
dolphmayoung: there's a recommendation to enable it in the config help, right?02:33
ayoungmorganfainberg, huh?  Old uuids are out there, recorded on other systems.02:33
jamielennoxit's not major - i'm just learning that some things shouldn't be public straight away02:34
dolphmjamielennox: target it to a bug / bp and point it at 0.6.x02:34
ayoungdolphm, um...no, just to not disable "revoke by id" without also enabling the events02:34
morganfainbergayoung, if you look at the example i gave, a short name (e.g. "adam" as the ldap bit) could net less than 32 bytes in the id: #SHORT:  c2hvcnTFkKCA4xVGhqHao2G41agj   2802:34
ayoungah...yeah, pad02:34
ayoungCOBOL!02:34
ayoungCCCCCCCCCCCDDDDDDDDD02:35
morganfainbergayoung, user_id - pad to 32 then encode?02:35
morganfainberghm...02:35
ayoungPad with spaces, I think02:35
morganfainbergayoung, yeah02:35
morganfainbergchecking that02:35
* dolphm bonus points for dishwashers that resume after power outages02:35
ayoungthen trim is part of the decode process02:36
morganfainbergayoung, yeah that works02:37
morganfainbergdolphm woo!02:37
jamielennoxdolphm: 0.6.1 or 0.7?02:37
jamielennoxdidn't we do 0.6.1?02:37
dolphmjamielennox: https://pypi.python.org/pypi/python-keystoneclient/02:37
jamielennoxyea, i checked that immediately after typing...02:38
dolphmhttps://launchpad.net/python-keystoneclient/+milestone/0.6.102:38
dolphmjamielennox: was auth plugins introduced in 0.6.0 or since?02:39
jamielennox0.602:39
jamielennoxahh, since 0.602:39
jamielennoxwe haven't had a release since02:39
dolphmjamielennox: is there a bp for it?02:39
jamielennoxlet me try again - there was some base plugin stuff in 0.6.0, the actual implementations have been since then02:40
jamielennoxhmm, there was02:40
jamielennoxi guess we can close that now02:40
dolphmjamielennox: you can point it to 0.7.0 and we can make that the next release?02:41
jamielennoxok02:41
dolphmjamielennox: delete 0.6.102:42
dolphmdeleted*02:42
dolphmayoung: did you have a patch for this? https://bugs.launchpad.net/cinder/+bug/128583302:45
ayoungdolphm, yep02:45
ayounghttps://review.openstack.org/#/c/77215/  dolphm I think that is the same issue02:45
dolphmayoung: your patch is referencing the wrong bug, use bug 128583302:46
ayoungOK02:46
stevemardolphm, yay limited trusts!02:48
ayoungdolphm, BTW, the KDS removal misses all these keystone/locale/kn/LC_MESSAGES/keystone.po:#: keystone/contrib/kds/common/exception.py02:48
ayoungis it in?02:48
dolphmstevemar: did it land?02:49
dolphmhttps://review.openstack.org/#/c/56243/02:49
dolphmgating02:49
stevemarit's going through02:49
*** marcoemorais has quit IRC02:49
ayoungdolphm, before I repost.... bknudson had this comment: (unrelated) how is an AssertionError or KeyError going to happen??   I am now cleaning up that code...but am reluctant to change the exception handling02:51
ayounghttps://review.openstack.org/#/c/77215/1/keystoneclient/middleware/auth_token.py02:51
bknudsonayoung: it was broken before so don't need to fix it.02:51
ayoungbknudson, kindof my feeling, but I refactored the two calls into a single function02:52
ayoungI guess I'll post and see what you think02:52
dolphmayoung: it's only your new code in the try block now -- you should feel comfortable changing it02:52
stevemardolphm, oh yeah - https://review.openstack.org/#/c/74531/02:52
ayoungdolphm, I don't remeber why those two exceptions, except that there was a lot of trial and error.02:53
dolphmstevemar: i thought that landed forever ago02:53
stevemardolphm, there were a few02:53
stevemardolphm, this one slipped through02:53
dolphmayoung: trial and error without tests?02:53
morganfainbergayoung, so is 32 bytes too little for the user_id derived element?02:53
morganfainbergayoung, http://paste.openstack.org/show/71990/02:53
ayoungdolphm, for fetching the certs?  Maybe.  It landed a long while back...2 years now?02:54
dolphmstevemar: +A02:54
stevemardolphm, ty02:54
jamielennoxayoung: yea i think that code has caught errors that don't exist for ages02:54
jamielennoxthere was a review a while ago where they were removed and it got negatively commented on and reinstated02:55
morganfainbergayoung, still seeing if i can figure out a better mechanism.02:55
ayoungmorganfainberg, 32chars for username should be OK.  Even with email addresses, that is pretty forgiving02:56
*** joesavak has joined #openstack-keystone02:56
dolphmayoung: both except blocks should be removed from the calling methods... if os.rename fails, a 500 wouldn't be so bad (it's a blatant misconfiguration)02:56
ayoungdolphm, true02:56
dolphmand getting a named temp file should always succeed02:57
ayoungKey error seems wonlky02:57
ayoungOK...it was moved over from server code...so the reason has been lost in antiquity02:58
ayoungAssertionError  is, I think, coming from the token parsing done earlier, probably to make the call to fetch the cert03:00
ayoungKey error, too03:00
ayoungwhich means that the block is useless03:00
dolphmjamielennox: bah, unset the approval on https://review.openstack.org/#/c/77701/03:07
dolphmjamielennox: the successful check was a lie...03:07
dolphmmorganfainberg: we're not running the config check_uptodate.sh on check jobs? :(03:07
morganfainbergwe should be03:07
dolphmmorganfainberg: https://review.openstack.org/#/c/77701/ passed check, and is now failing gate03:07
dolphmhttps://jenkins07.openstack.org/job/gate-keystone-pep8/371/console03:07
*** amcrn has joined #openstack-keystone03:08
*** joesavak has quit IRC03:11
ayoung-pep8  probably due to config file merge03:13
ayoungcan we drop that check?03:13
ayoungno, it just probably merged with something else.   check_uptodate.sh  is too strict03:14
*** dstanek has quit IRC03:24
*** dstanek_afk has joined #openstack-keystone03:24
*** ChanServ sets mode: +v dstanek_afk03:24
*** derek_c has joined #openstack-keystone03:26
*** gokrokve has joined #openstack-keystone03:27
*** dstanek_afk is now known as dstanek03:27
dstanekit looks like there is only 1 review left for i3?03:27
ayoungmine?03:29
dstaneklooks like it - walking through it again now03:30
ayoungthanks03:30
dolphmsomeone double check me on this... gate failed https://review.openstack.org/#/c/77701/ on the new config check job, which tried to import keystone.openstack.common.sslutils for config opts https://jenkins07.openstack.org/job/gate-keystone-pep8/371/consoleFull03:32
dolphmabsolutely nothing uses that module (not even other oslo code)03:32
dolphmit's not referenced in openstack-common.conf03:32
dolphmgit blame shows it was added in an irrelevant (i think?) patch, and hasn't been touched since https://review.openstack.org/#/c/41487/03:33
dolphmmorganfainberg: ^ and i still don't understand why the check job would have passed03:34
dolphmit was easy to repro offline03:34
ayoung find keystone/ -name \*py | xargs grep sslutil  finds nothing03:34
ayoungdolphm, config/generator.py"03:35
dolphmmorganfainberg: (check_uptodate.sh ran and exited successfully in the check job, but failed in gate)03:35
ayoungcould that be called in by  check_uptodate.sh03:35
ayounglike an old one?03:35
dolphmtools/config/generator.py?03:36
morganfainbergdolphm, hm03:36
ayoung.tox/py27/lib/python2.7/site-packages/oslo/messaging/_drivers/impl_rabbit.py:from oslo.messaging.openstack.common import sslutils03:36
dolphmayoung: tools/config/generate_sample.sh ?03:36
morganfainbergdolphm, the check_sample stuff is dumb and catches .pyc files03:36
dolphmmorganfainberg: but, in zuul?!03:37
morganfainbergdolphm, it looks like something tried to push an empty sample_config03:37
morganfainbergwhat....03:37
*** chandan_kumar has joined #openstack-keystone03:37
morganfainbergsslutils error could do a number on it03:37
dolphmmorganfainberg: sslutils bailed with stderr, while stdout from the generator was being redirected to keystone.conf.sample maybe?03:38
morganfainbergdolphm, possibly03:38
morganfainbergdolphm, therefore the "sample" it was comparing against was effectively empty03:39
dolphmmorganfainberg: seems like the order of the diff should be reversed then03:39
dolphmmorganfainberg: it should be -every line not +every line03:39
ayoungcan we drop the check for now?  I think we've caught up with the config.py file03:39
morganfainbergdolphm, checking right now before we drop that check03:40
dolphmayoung: morganfainberg: why do we want to drop the check?03:40
ayoungwe just for the next couple of days03:40
ayoungdolphm, it causes a lot of rebase errors03:40
morganfainbergdolphm, if we can't solve it :)03:41
morganfainbergdolphm, i'd rather not03:41
dolphmayoung: rebase errors?03:41
ayoungif two patches run the tool, the resultant merge doesn't pass the check03:41
morganfainbergdolphm, so i see a legitimate error w/ the sslutils options03:41
dolphmmorganfainberg: ?03:42
morganfainbergiif i checkout your patchset03:42
ayoungso if something merges after I submit, even if my patch passed check the first time,  the auto-rebase on gerrit fails the second03:42
morganfainbergdolphm, http://paste.openstack.org/show/72004/03:42
dolphmayoung: that just sounds like bad luck... have an example?03:42
ayoungdolphm, well, my patch has gone through 20 reivsions in about 4 dyas...several because of that03:43
morganfainberghrm03:43
morganfainberglet me try a quick rebase03:43
dolphmmorganfainberg: is that the latest patchset? or the one that failed gate?03:43
morganfainbergyeah03:43
morganfainbergdolphm, give me a sec trying a rebase03:43
ayoungwe can re-enable it once we get the tool stable03:44
morganfainbergdolphm, yep, looks like the keystone.openstack.common.sslutils stuff is causing issues03:44
dolphmayoung: better yet, let's stop landing features for the next couple weeks so no new config options will be introduced :D03:45
morganfainbergdolphm, ah because you removed them without regenerating the sample03:45
ayoungdolphm, heh.  OK.03:45
dolphmmorganfainberg: am i regenerating the sample wrong??03:45
ayoungdolphm, maybe the tool picked up stray .pyc files03:45
ayoungdo a git clean03:45
morganfainbergdolphm, your patch doesn't have a sample config in it03:45
ayounggit clean -xdf keystone03:46
morganfainbergdolphm, keystone.conf.sample03:46
dolphmmorganfainberg: your docs don't say how to actually generate the sample :P http://docs.openstack.org/developer/keystone/developing.html#generating-updated-sample-config-file03:46
morganfainbergtox -esample_config -r should do it03:46
morganfainbergbut if you have .pyc files03:46
morganfainbergyeah should say you need to do a git clean -fdx03:46
morganfainbergor at least remove all .pyc files to be 100% sure03:46
morganfainbergeven files not being tracked by git, if they provide options, will be caught03:47
morganfainberg:(03:47
morganfainbergsorry :(03:47
dolphmmorganfainberg: i remove pyc files, actually...03:47
ayoungwhat are we doing now to activate things in paste.ini for tests that are not in etc/keystone-paste.ini03:48
dolphmayoung: there's a config generator function that can insert things into the pipeline03:49
morganfainbergayoung, i think the oauth and endpoint_filtering stuff has those semantics in it03:49
dolphmayoung: it basically does a find/replace on the default pipeline03:49
morganfainbergyep03:49
dolphmayoung: look in keystone.tests.core for 'generate'03:49
ayoungonly in extension code, though?03:49
dolphmmorganfainberg: okay, trying to regenerate conf again... did an ls to ensure no *.pyc first03:50
ayounghmmm...I might have trouble with that if I remove the revoke one.03:50
ayoungI would need to pass in multiple extensions03:50
dolphmayoung: like what?03:51
morganfainbergdolphm, you should see the sslutils opts disappear03:51
ayoungdolphm,   oauth events03:51
ayoungmaybe not...03:53
ayoungnah, those are just testing for generation of events03:53
morganfainbergdolphm, looks like some people went through and "fixed" a bunch of strings as well03:53
morganfainbergwith periods at the end of them (maybe oslo.messaging)03:53
morganfainbergdolphm, i have a03:54
morganfainberg"fixed" patchset if you want me to try03:54
morganfainbergif not i can let you publish03:54
dolphmmorganfainberg: for every code review, i git checkout $BRANCH && git pull && git checkout $BRANCH~0 && find . -name "*.pyc" -delete && git review [-x|-d] $REVIEW_NUMBER where $BRANCH is usually 'master'03:54
morganfainbergdolphm, that should be fine03:54
morganfainbergdolphm, no reason it should cause issues03:55
jamielennoxdolphm: :O, you have that as a script or something i take it03:55
jamielennoxhttp://www.jamielennox.net/blog/2014/02/18/dealing-with-pyc/03:56
dolphmmorganfainberg: no success http://pasteraw.com/a3lj19yywkwh8dfx0ybjlx0lqrllvqo03:56
dolphmjamielennox: script is in there ^03:56
jamielennoxdoesn't cover everything of that03:56
morganfainbergdolphm, so clean venv, git clean -fdx and nothing lingering around03:57
morganfainbergdolphm, http://pasteraw.com/h1e5capcn76y8dyaa61t5pakz7858ad03:58
lbragstadayoung: getting down the nitty gritty here, https://review.openstack.org/#/c/55908/03:59
dolphmmorganfainberg: i ran rm_pyc again and then http://pasteraw.com/bthcfj4t85bd55yc7afl78ck2r1cb5503:59
morganfainbergdolphm, and i ran tox -esample_config -r to generate that03:59
morganfainbergyeah that should be fine.03:59
morganfainbergdolphm, WTF.03:59
lbragstadayoung:  you mind if I checkout that patch and go through all the indentation and comment '#<space>' fixes?03:59
ayounglbragstad, dagnabit, I know I changed those return codes in an earlier patch.03:59
dolphmlbragstad: can you get that into openstack/hacking :D04:00
ayounglbragstad, yeah, but let me get an updated review04:00
morganfainbergdolphm, lbragstad ++ in hacking!04:00
lbragstadoh, the consistent04:00
lbragstad# comment check?04:00
ayoungI'm testing out disabling the extension by default, but need to update a few tests first04:00
lbragstadhell yeah, I'll check it out... that would be really... really nice04:00
morganfainbergdolphm, not sure why we're getting wildly different results here04:01
dolphmlbragstad: i started coding it myself but didn't get very far http://pasteraw.com/rsi2frll7ojzr1b5bhj6si6mkbf7e8y04:01
dolphmlbragstad: i'm not sure what's useful in that patch04:02
lbragstaddolphm: cool, I'll check it out.04:02
* lbragstad bookmarks04:02
dolphmmorganfainberg: go ahead and post a revised patchset04:04
morganfainbergdolphm, ok04:04
dolphmmorganfainberg: wow... you weren't kidding about the periods04:05
morganfainbergdolphm, ok i think we need to change how the sample config is done.04:05
morganfainberglets do it like we do man pages04:05
morganfainbergdisable it from pep8, and just do it once every X period04:06
morganfainberglike before we cut an rc.04:06
lbragstadayoung: you're posting another token revocation patch right?04:06
morganfainbergmake it a bug ticket that needs to be the last one to land until it gets more automated04:06
morganfainbergs/rc/milestone04:06
dolphmmorganfainberg: i'd rather man pages be done more like this... is nova approaching this any differently?04:06
ayounglbragstad, I have a bunch of nits....and I am also looking to see how hard it would be to get the tests running if we disable the extension by default04:06
lbragstadayoung: because I can go through the nits I made04:07
morganfainbergdolphm, they are supposed to be automating it for deployment04:07
lbragstadwhile you work on other things, if that helps04:07
morganfainbergdolphm, e.g. when you cut a release, this is done04:07
morganfainbergdolphm, but no idea where that is or how hard it is to do atm04:07
ayounglbragstad, if you have a cleaned copy of the patch, you can post it and I can merge, or you can post comments....I'm guessing you have a pretty extensive review?04:07
ayoungOh, wait, I confused you with bknudson , nevermind04:08
lbragstadlol04:08
lbragstaddamn... i was going to take credit! :)04:08
morganfainbergdolphm, maybe we should get a jenkins task to do the update? same as translations/requirements?04:08
morganfainbergdolphm, same w/ man page?04:08
ayoungso...to disable, is it OK to leave the filter definition in, and just remove the filter from the pipeline?04:09
dolphmmorganfainberg: i'd be down with that04:09
dolphmmorganfainberg: on both counts04:09
morganfainbergdolphm, i'll get something whipped up and proposed to infra for us (hey maybe others will want it too)04:09
dolphmmorganfainberg: as long as it's automated in some fashion -- bonus points if jenkins proposes commits on top of the commits that are causing the changes rather than waiting for master :P04:09
morganfainbergdolphm, was going to do it like the translations, a periodic that is just run.04:10
morganfainbergdolphm, if there is a change, propose it04:10
dolphmmorganfainberg: ah, i thought translations was triggered04:10
morganfainbergdolphm, nope, don't think so.04:10
dolphmmorganfainberg: either way04:10
morganfainbergdolphm, but they only run once a week or so, requirements is triggered afaik04:10
* dolphm need tips on horizontally scaling sqlite in production thx04:11
morganfainbergdolphm, OH! i stopped using SQLite so i could use mongodb, it's webscale04:11
dolphmmorganfainberg: but is mongodb lite? this needs to fast04:12
morganfainbergdolphm, i don't know if it's lite...but i hear it's ACID compliant04:13
dolphmlol04:13
dolphmmorganfainberg: can you back mongodb to existing sqlite deployment?04:14
morganfainbergdolphm, I love it!04:14
morganfainbergdolphm, we need to do that.04:14
dolphmmorganfainberg: is it problem if sqlite db is 6.8GB? how do you enable sharding04:18
dolphm-rw-r--r-- 1 keystone keystone 6.8G Mar  4 04:17 /var/lib/keystone/keystone.db04:18
morganfainbergdolphm, LOL04:18
morganfainbergdolphm, thats an awesome SQLite db04:19
morganfainbergdolphm, wow.04:19
ayoungthe sad thing is that people wouldn't know you were joking without the LOL04:20
morganfainbergayoung yeah04:20
morganfainberg=/04:21
lbragstadayoung: here are the fixes I made,04:21
lbragstadhttp://paste.openstack.org/show/72011/04:21
lbragstadayoung:  ready to push for review if its cool with you04:21
ayoungdolphm, OK,  so disable revoke_events by default doesn't look too bad...just had to add the extension to a couple tests.  Running the whole thing now04:21
ayounglbragstad, looking04:21
ayounglbragstad, do me a favor:  git format-patch it and mail me?  I'll integrate it into my next version04:22
ayoungdolphm, why does building docs fail with sphinx.errors.SphinxWarning: /opt/stack/keystone/doc/source/api/keystone.openstack.common.notifier.rst:7: WARNING: autodoc can't import/find module 'keystone.openstack.common.notifier', it reported error: "No module named notifier", please check your spelling and sys.path04:23
morganfainbergayoung, .pyc files?04:23
dolphmayoung: curl http://paste.openstack.org/raw/72011/ | git apply04:23
morganfainbergor sphinx is trying to load / configured to read things from there?04:23
ayoungmorganfainberg, I've git cleaned...or are those somehow embedded in the venv04:23
ayoungdolphm, thanks....04:24
morganfainbergdolphm, oh.04:24
morganfainbergdolphm, ick04:24
morganfainbergdolphm, eyah04:24
dolphmmorganfainberg: are you convulsing?04:24
morganfainbergdolphm, kfjasfjkskf122131231adf04:24
morganfainbergdolphm, maybe04:24
morganfainbergdolphm, makes me cry inside.04:25
ayoungfatal: corrupt patch at line 18504:26
dolphmayoung: that's a conflict with your own changes04:26
ayoungdolphm, yeah...I can do it as a cherry pick04:27
*** wchrisj has quit IRC04:27
dolphmayoung: could also try git apply -C [0,1,2] (reduce context)04:28
dolphm-C1 (no space)04:28
dolphmayoung: you can also --exclude files you've already made changes to04:29
lbragstadayoung:  did you get it?04:29
ayounglbragstad, not yet04:29
dolphmayoung: and lastly, --recount has fixed other contextual issues for me04:29
*** harlowja is now known as harlowja_away04:30
ayoungdolphm, used an oldey but gody04:31
ayoungpatch -p1 < /tmp/lbrag.patch04:31
ayoungdolphm, do you want the extension disabled by default?  Let me know before I try to merge his with mine04:32
ayoungI'm guessing Yes at this point, though04:32
dolphmayoung: yeah :-/ as much as i think we should **strongly** recommend everyone enable it immediately04:33
ayoungdolphm, we can always flip the switch on it once it is tested04:34
ayoungwe need  client support for it to be useful anywa04:34
ayoungy04:34
dolphmayoung: ++04:34
ayoungOK...I'll submit that way04:34
*** harlowja_away is now known as harlowja04:35
lbragstadwoo vagrant dir ftw04:35
dolphmbest devstack run ever http://logs.openstack.org/15/77215/2/check/gate-tempest-dsvm-large-ops/953025e/console.html04:35
*** wchrisj has joined #openstack-keystone04:36
ayounglbragstad, reposting.  Please confirm your changes were made clean04:36
lbragstadayoung: will do04:37
ayounglbragstad, I didn't rebase prior to posting....04:37
* ayoung getting tired04:37
ayoungmorganfainberg, stevemar BTW, I think I have a new record for number of revisions on a patch.  Oauth went to 64.  I'm pushing 8004:40
* stevemar passed the torch back to ayoung04:41
stevemarpasses*04:42
stevemariirc you had it for trusts, at 61?04:42
ayoungstevemar, that is not including revisions on the two patches I merged in to this.04:42
ayoungtrusts was about that...and the tests were separate, but dolphm wrote those04:42
stevemarayoung, details, details04:42
* ayoung not writing any big security related patchsets ever again 04:43
dstanekwhat was the veridict on removing existing openstack copyrights?04:44
ayoungah...right04:45
ayoungdstanek, let me see if I wrote that in the first place, in which case its moot, no?04:45
ayoungdstanek, which file?04:45
dolphmdstanek: leave existing ones alone -- just don't add new ones for now04:45
dolphmdstanek: existing ones need to be removed all at once, and probably by the foundation04:46
dstanekayoung: :-) i don't know anymore...jas let me see if i can find it04:46
ayoungpatch set 7104:47
dstanekhttps://review.openstack.org/#/c/55908/77/keystone/tests/test_v3_auth.py04:47
dstanekayoung: ^04:47
ayoungah, that is likely dolphm 's work04:48
ayoungdstanek, OK, think that is the only one I butchered, but I'll double check04:49
ayoungahhh!04:50
ayoungpatching errors04:50
ayoungdstanek, OK, that was the only copyright change in the patch...aside from accidentally adding in keystone/token/controllers.py.orig  which will be gone in the next revision04:51
lbragstadayoung: there were a couple things i missed or they were new in your patch04:51
ayounglbragstad, you commented on the erroneous .orig file04:51
ayounghttps://review.openstack.org/#/c/55908/78/keystone/token/controllers.py.orig04:51
ayoungthat was an artifact of doing the patch, and some fuzz04:52
lbragstadayoung: ahh... yep, you're right04:52
ayounglbragstad, I think all of the changes made it in to https://review.openstack.org/#/c/55908/78/keystone/token/controllers.py04:52
dstanekayoung: just commented on it04:53
ayoungdstanek, integrating your changes now04:53
lbragstadayoung: yeah ^^ that one looks right04:53
*** chandan_kumar has quit IRC04:53
ayoungdstanek, II'm assuming the loop in downgrade is not essential to change?04:55
dstanekno, at least not to me - it's just funny looking04:55
dstaneki think the exit (or lack of exit) is what really needs addressed04:55
ayoungdstanek, roles = token_data.get('metadata', {}).get('roles', [])04:55
ayoungif there is no metadata...will there be roles?04:55
ayoungeasy to test04:56
ayoungOK...I guess roles can't be none04:56
dstanekayoung: with a lack of keys that should give you an [], but is there a case there the dict will actually contain a None?04:56
ayoungdstanek, ,metadata will never be None if that is what you mean?04:57
dolphmayoung: "metadata" isn't supposed to be in the token04:57
ayoungdolphm, V2?04:58
dolphmayoung: "roles" are in "user", and "metadata" is a bug04:58
ayoungah.....04:58
ayoungdolphm, so04:58
dolphmessex or folsom leaked data from the backend into the token, and it stuck04:58
dolphmwhich is where "extra" came from as well04:58
ayoung roles = token_data.get('user', {}).get('roles', [])04:58
dolphmand then gyee added it to v3 *intentionally*04:58
dolphmayoung: that looks right, but it's role names, i think?04:59
dolphmayoung: one is IDs, the other is names04:59
ayounglooking04:59
ayoungnope roles05:00
ayounghttps://github.com/openstack/keystone/blob/master/keystone/token/providers/common.py#L50  dolphm05:00
dolphmayoung: i mean the values in that list are names05:00
ayoungsame data is used for both user and metadata sections05:01
dolphmayoung: they're passed in to that method separately05:02
dolphmayoung: metadata comes out of token_ref, and roles_ref is passed in explicitly05:03
dolphmjamielennox: still around?05:04
jamielennoxdolphm: yea05:04
jamielennoxhaven't been following05:04
dolphmjamielennox: morganfainberg's revision to https://review.openstack.org/#/c/77701/ is about to pass jenkins check, needs another +A05:04
stevemardolphm rgr05:05
jamielennoxdolphm: i'll put a +a on it when jenkins passes05:05
dolphmjamielennox: danke05:05
dolphmi'm off to bed - night everyone05:07
lbragstadlater05:08
morganfainbergdolphm, https://review.openstack.org/#/c/77789 https://review.openstack.org/#/c/77790/05:11
morganfainbergdolphm, lets see what infra has to say about it05:11
morganfainbergayoung, jamielennox, ^05:12
jamielennoxmorganfainberg: failed already :(05:13
morganfainbergjamielennox, typo in yaml05:13
morganfainbergok typo fixed05:17
*** wchrisj has quit IRC05:17
ayoungcrud...looks like in V2 we only get role names...05:18
ayoungwe don't have any tests that are specific on the roles being revoked for v2, so it was slipping through.  I was pulling the values out of the wrong part of the token05:20
ayoungdstanek, nice catch05:20
ayoungdo I need to add role_name to the revocation API?05:21
ayoungdolphm, you still awake by any chance?05:22
morganfainbergayoung, really only role name?05:22
morganfainbergayoung, doh!05:22
ayoungyep05:22
morganfainbergayoung, when v2 goes away.......05:22
ayoungmorganfainberg, in both metadata and in user section05:23
morganfainbergayoung, bleh, prob need to revoke by role name then as well.05:23
ayoungwait...maybe05:23
ayoungit might be Id in metadata05:23
morganfainbergayoung, that would make life a bit better05:23
ayoungyehaa!05:23
morganfainbergthis is why i ask these questions05:23
ayoungI need a test for that, though05:24
ayoungshould have been caught by existing tests05:25
morganfainbergayoung, ++05:25
ayoungmorganfainberg, I'm guessing that revoking a role is doing something much more draconian, and that is masking it05:25
morganfainbergayoung, probably05:26
morganfainbergayoung, actually didn't that do something horrible like revoke all tokens for that user (at least at one point)05:26
ayoungyeah05:26
morganfainbergayoung, https://review.openstack.org/#/c/77790/05:28
morganfainbergayoung, hopefully that gets looked at / in soon05:28
morganfainbergthen pep8 on sample config goes away05:28
stevemarjamielennox, jenkins passed https://review.openstack.org/#/c/77701/05:31
stevemaryou can do the honors05:31
jamielennoxstevemar: done thanks05:32
dstanekmorganfainberg: that's really nice05:33
morganfainbergdstanek, ayoung, stevemar, so an alternative to the user_id stuff: http://paste.openstack.org/show/72032/ I'm not pleased with the result...but it's better than user_id@@domain (no chopping needed to fit everything). it still suffers from 32 bytes being the limit for the user_id derivation05:35
ayoung640k should beenough for anyone05:36
morganfainbergdstanek, ayoung, stevemar, after spending a good deal of time talking it over here internally, we [my coworkers and I] feel that 32 bytes is just going to be too restrictive. I'll back off on my "you're wrong" if this is the only way, but honestly, I think we're going to run into issues with data for user_id, and the map table is a better fit05:37
ayoungmorganfainberg, we have been restricted to 32 bytes thus far05:37
ayoungbut...05:37
morganfainbergayoung, we haven't been sourcing data from federated users and SAML assertions05:37
ayoungsend it past henrynash, he's the one with the burning need for multiple LDAP05:38
morganfainbergyeah.05:38
*** gokrokve has quit IRC05:38
morganfainbergayoung, the real win here is just that we know the fixed fields and get a nice(who am i kidding, b64 is ugly) user_id that is urlsafe05:38
ayoungmorganfainberg, I'd also be interesting in Chadwicks perspective05:39
*** gokrokve has joined #openstack-keystone05:39
ayoungmorganfainberg, I'm not sure that the effort is worth it05:39
morganfainbergayoung, without regressing or needing to update field lengths everywhere in OS05:39
ayoungI'd rather get a longer userid than worry about the urlsafety05:39
stevemarayoung, yeah, chadwicks opinion would be good on this issue05:39
morganfainbergayoung, you're not going to get a longer user_id05:39
ayoungI think urlsafety is going to be provided05:39
ayoungmorganfainberg, yes..if we do a really short domain id05:40
morganfainbergayoung, i do magic to raw bytes to cut down the size of the domain_id05:40
ayoungdrop the uuencode05:40
ayounguserid@@12345605:40
morganfainbergayoung, can do the same thing w/ this code and maintain url_safety05:40
ayoungbut ^^ gets you close to 60 bytes05:40
derek_cis there a way to log the HTTP requests/responses sent/received by keystoneclient?05:40
morganfainbergayoung, i'm not going to budge on urlsafe. sorry.05:41
jamielennoxderek_c: using the standard python logging05:41
jamielennoxit'll print to debug05:41
jamielennox--debug if you mean the CLI05:41
morganfainbergayoung, asking everyone who interacts with the API to urlencode when we could sanitize is really dumb05:41
morganfainbergayoung, and terrible ux05:41
ayoungmorganfainberg, no...you are missing the point05:42
morganfainbergand since we don't control what is coming in from SAML or ldap bits...05:42
ayoungthe IDS that come in *have* to be URL safe already05:42
ayoung its not for us to provide an additional layer on top of that05:42
morganfainbergayoung, how are you ever enforcing that?05:42
derek_cjamielennox: thanks! that was helpful05:42
ayoungmorganfainberg, its on the LDAP admins to do it05:42
morganfainbergayoung, "oopse this user can't use OpenStack because they have an unsafe element"05:42
ayoungmorganfainberg, heh..funny story05:43
ayoungso there is a Woman at BU with the username Nova....05:43
*** gokrokve has quit IRC05:43
morganfainbergayoung, you're asking a lot. we can't guarantee someone will change their schema for us, how can we guarantee they wont do something non-url safe?05:44
morganfainbergayoung, let alone whatever falls out of the SAML assertion05:44
morganfainbergayoung, hell, we can't even write to many ldap deployments, - admins loathe to change things for applications05:44
morganfainbergayoung, anyway, we'll run this by henry and dolph tomorrow05:45
ayoungmorganfainberg, if a user has a non urlsafe username, none of the web apps work.05:45
ayoungIts like a rule or something05:46
*** stevemar has quit IRC05:46
*** bvandenh has joined #openstack-keystone05:49
ayoungmorganfainberg, but ... url encoding actually won't be that expensive, will it?  Its like, what 3 extra charaters for each non url safe char?05:51
derek_clooking at the keystoneclient code, does anyone know why some APIs use URLs like /users/%s/OS-KSADM/enabled?  What's that OS-KSADM for?05:53
ayoungIt was an extension that let you do non-core stuff05:53
jamielennoxderek_c: by convention anything that starts with an OS- is an extension05:53
ayounghttps://github.com/openstack/keystone/blob/master/keystone/contrib/admin_crud/core.py  derek_c05:54
ayoungderek_c, it was before the V3 api, and alot of the basic crud operations were missing05:54
morganfainbergayoung, it's 50% total more space or so.05:54
morganfainbergayoung, oh w/o b64 sure05:54
morganfainbergayoung, not sure how friendly urlencoded is but it's a blob userid05:55
ayoungmorganfainberg, yeah, b64 is only for binary05:55
ayoungmorganfainberg, so...would that satify your craving for url-safety goodness?05:55
morganfainbergayoung, i'm concerned about going to too short a domain.05:55
derek_cinteresting. I'm working on a contrib for the V3 api.  so should I also use a URL like this?05:55
morganfainbergayoung, but sure urlencode would be fine.05:55
morganfainbergayoung, and as long as we keep enough bytes (how efficient is the SQL for searching on the chopped domain?) i'm fine with that05:56
ayoungdomain id?  Can be short, and we can even record it if we worry about conflicted.  6 chras off the front of the UUID for the domain by deffault05:56
morganfainbergayoung, yeah i figure 12 chars is about right05:56
morganfainbergmaybe 1005:56
morganfainbergi still think even if we went 10, with 52 bytes we're going to be cutting the id REALLY close to the limits05:57
morganfainberg52 bytes isn't much room.05:57
morganfainbergayoung, i'm still in favor of the map table. but lets see what henry has to say and dolph tomorrow05:57
ayoungmorganfainberg, if we go with mapping, we do it in the existing user table05:58
morganfainbergayoung, not opposed to something like that05:58
morganfainbergayoung, but i understand why you don't want it. it makes sense.05:58
ayoungmorganfainberg, no. But I am.05:59
ayoungheh...let me get this thing working...05:59
morganfainbergayoung, back to revocations.05:59
morganfainbergayoung, :P05:59
morganfainbergayoung, tomororw we will chat w/ dolph and henry see what makes sense if anything05:59
ayoungyeah...so the V2 tests by pass the code that triggers the event...05:59
ayounggoes right to the underlying API05:59
morganfainbergayoung, oh joy05:59
morganfainbergayoung, :(05:59
morganfainbergayoung, somehow not surprised06:00
ayoungI can fix it...I have the technology06:00
morganfainbergayoung, a text editor and unit test framework?06:01
ayoungand the web API06:01
*** topol has quit IRC06:01
*** marcoemorais has joined #openstack-keystone06:01
morganfainbergayoung, might need scotch to go with that06:01
ayoungmy dad is a television repairman, he's got the ultimate set of tools.  I can fix it.06:01
ayoungHeh, I might need that OS_KASDMIN extension for this06:02
* lbragstad perks at scotch 06:04
derek_cdoes the keystoneclient only support 2.0 APIs?06:05
jamielennoxderek_c: the CLI - yes06:05
derek_cjamielennox: hmmm06:06
derek_cjamielennox: the code is there though. why don't we use it?06:06
jamielennoxwe are switching to openstackclient for v3 cli06:06
lbragstadderek_c: you can use the openstack-client for some of the other v3 related stuff though06:06
jamielennoxthe keystoneclient is useful for more than just the CLI06:06
jamielennoxplenty of python applications will want to use the library directly06:06
lbragstadosc will forward to keystone-client06:07
derek_cok I see. but if I'm hacking on v3-related stuff, I guess I should add the code to openstackclient?06:08
jamielennoxderek_c: you will want to add the action code to keystoneclient and then the CLI code to osc06:09
jamielennox(although i'm not sure if they are using us at the moment - check first)06:09
ayoungOK...so a lot of the role_assignments are going to slip through right now...06:10
*** bvandenh has quit IRC06:11
*** henrynash has joined #openstack-keystone06:11
ayoungwe don't emit notifications for role-assignment changes.06:11
derek_cjamielennox: I see, thanks!06:11
morganfainbergayoung, hmm.06:12
morganfainbergayoung, v3 do we or all role assignments?06:12
morganfainbergayoung, slip through that is?06:12
ayoungnah, all of those were happening in the controllers06:12
morganfainbergayoung, ah ok06:13
morganfainbergayoung, so role assignments (addition or both addition/removal) in v2?06:13
morganfainbergbecause if it's addition, i don't care too much06:13
ayoungno, removal06:13
morganfainbergayoung, boo :(06:13
ayoungremove role from user and tenant06:13
*** amcrn has quit IRC06:13
morganfainbergayoung, hmmmm06:13
ayoungself.assignment_api.remove_role_from_user_and_project(06:13
ayoungwe don't have a notification for that06:14
* morganfainberg looks06:14
ayoungso I need to call revoke_api directly for now06:14
morganfainbergayoung, i think that is a reasonable alternative.06:14
morganfainbergjust add it into the manager.remove_role_from_user_and_project ?06:15
ayoungum wait...06:15
*** harlowja is now known as harlowja_away06:16
morganfainbergit's the same place we do revoke_tokens for that.06:16
ayoungah..yeah, trest is now failing for the old API...which I need to make pass again.06:16
ayoungnah...too much to get done tonight, I suspect06:16
*** lbragstad is now known as lbragstad_06:18
derek_cI'm trying to use the tools/install_venv.py in openstackclient but got a bunch of errors when installing the cryptography package.  the errors are basically the same as described here: http://stackoverflow.com/questions/22073516/failed-to-install-pyhton-cryptography-package-with-pip-and-setup-py06:19
derek_cdoes anyone know what could be going on?06:19
morganfainbergderek_c, same errors as that SO post?06:20
morganfainbergderek_c, or different, that one looks like cffi lib is the wrong version06:20
morganfainbergayoung, i'll do another pass on the revocation stuff before i head home in an hour here06:21
morganfainbergayoung, i don't think i had anything outstanding but i've looked at it a lot06:21
derek_cmine also seems to be cffi-related06:21
derek_cseems to die at: c/_cffi_backend.c:14:17: fatal error: ffi.h: No such file or directory06:21
ayoungmorganfainberg, thanks...I'll direct yo uat the V2 specific stuff06:21
morganfainbergayoung, thanks :)06:21
morganfainbergderek_c, ah you need the devel libraries for ffi06:22
derek_clibffi-dev?06:22
morganfainbergderek_c, that sounds right06:22
derek_cmorganfainberg: cool I will give it a try. thanks!06:22
morganfainbergderek_c, sure thing!06:22
ayoungmorganfainberg, OK, its not so bad...just scary that it got this far, but now I see how I got fooled06:26
morganfainbergayoung, np.06:26
*** gokrokve has joined #openstack-keystone06:27
morganfainbergayoung, it happens :P06:27
morganfainbergit's why we code review massive changes into the ground.06:27
morganfainbergactually, it's why we don't do massive code reviews (not that it was easily avoidable here)06:27
morganfainbergayoung, crud, i was going to add something to the meeting agenda...06:28
morganfainbergnow i can't remember what it is06:28
*** gokrokve_ has joined #openstack-keystone06:29
ayoungmorganfainberg, there was no choice with this one.06:29
morganfainbergayoung, yep.06:29
morganfainbergayoung, it wasn't exactly bite sized bits being implemented06:29
*** gokrokve has quit IRC06:31
*** henrynash has quit IRC06:34
*** gyee has quit IRC06:34
*** henrynash has joined #openstack-keystone06:56
*** henrynash has quit IRC06:58
*** henrynash has joined #openstack-keystone07:00
ayoungmorganfainberg, https://review.openstack.org/#/c/77411/  one patch, no rechecks.  THat may be a record07:08
morganfainbergayoung, hehe07:09
morganfainbergayoung, no rechecks is the part that makes me go "what happened here"? :P07:09
ayoungmorganfainberg, just reposted https://review.openstack.org/#/c/55908/07:09
morganfainbergayoung, ah was actually in the middle of reviewing :P07:10
ayoungthe part that really needs a deep look is the changes to assignments/core07:10
morganfainbergayoung, ok no worries. i'll pull forward any comments i have07:10
morganfainbergnbd07:10
ayoung'salright...diff is pretty clear07:10
ayoungI'll look for comments in each review07:10
morganfainbergayoung, nah some were related to dstanek's i'll just pull em all forward only had 1 or two so far07:11
ayoungthe new test in test_content_types is important.  It is what really tests the revoke on v2 tokens07:11
morganfainbergk07:11
morganfainbergthanks07:11
ayoungI'm a crash07:12
ayoungits past 2...and we still will havea a7 AM wakeup with the kids07:12
*** saju_m has joined #openstack-keystone07:12
morganfainbergsee ya07:12
*** ayoung is now known as ayoung-ZZzZzzZz_07:12
*** bvandenh has joined #openstack-keystone07:30
*** henrynash has quit IRC07:32
*** bvandenh has quit IRC07:36
*** morganfainberg is now known as morganfainberg_Z08:02
*** henrynash has joined #openstack-keystone08:03
*** henrynash has quit IRC08:22
*** saju_m has quit IRC08:26
*** morganfainberg_Z is now known as morganfainberg08:44
*** morganfainberg is now known as morganfainberg_Z08:46
*** amichon has joined #openstack-keystone08:54
*** leseb has joined #openstack-keystone08:57
*** derek_c has quit IRC08:57
*** arborism has joined #openstack-keystone09:04
*** amichon has quit IRC09:06
*** amichon has joined #openstack-keystone09:06
*** arborism is now known as amcrn09:07
*** marcoemorais has quit IRC09:19
*** raies has quit IRC09:30
*** saju_m has joined #openstack-keystone09:42
*** leseb has quit IRC10:07
*** leseb has joined #openstack-keystone10:09
*** morganfainberg_Z is now known as morganfainberg10:39
* morganfainberg wonders if about the Nova v2 conversation(s)10:40
*** amcrn has quit IRC11:04
*** amcrn has joined #openstack-keystone11:13
*** morganfainberg is now known as morganfainberg_Z11:16
*** leseb has quit IRC11:18
*** leseb has joined #openstack-keystone11:19
*** leseb has quit IRC11:23
*** marekd|away is now known as marekd11:43
*** amichon has quit IRC11:57
*** leseb has joined #openstack-keystone12:01
*** leseb has quit IRC12:05
*** amichon has joined #openstack-keystone12:23
*** leseb has joined #openstack-keystone12:30
*** david-lyle has quit IRC12:39
*** topol has joined #openstack-keystone12:39
*** saju_m has quit IRC12:40
*** zhiyan has joined #openstack-keystone12:43
*** jamielennox is now known as jamielennox|away12:50
*** saju_m has joined #openstack-keystone12:52
*** achampion has quit IRC12:57
*** saju_m has quit IRC13:04
*** bvandenh has joined #openstack-keystone13:08
*** bvandenh has quit IRC13:18
*** bvandenh has joined #openstack-keystone13:18
dstanekdolphm, morganfainberg_Z, ayoung-ZZzZzzZz_: i just update the token revocation review to fix minor things Morgan and I brought up13:53
dstanekthere are still a few things on it that may need to be addressede13:54
*** wchrisj has joined #openstack-keystone14:08
*** achampion has joined #openstack-keystone14:09
*** lbragstad_ has quit IRC14:18
*** amichon has quit IRC14:26
dolphmdstanek: thanks! looking now14:27
dstanekdolphm: i thought his comment about optional dependencies may need some discussion14:28
dolphmdstanek: where at?14:28
dstanekdolphm: he mentions it pretty much everywhere revoke_api is being injected14:29
*** richm has joined #openstack-keystone14:32
*** dims has quit IRC14:43
*** nkinder has quit IRC14:45
*** lbragstad has joined #openstack-keystone14:45
*** dims has joined #openstack-keystone14:46
*** huats has quit IRC14:46
*** huats has joined #openstack-keystone14:47
*** huats has quit IRC14:47
*** huats has joined #openstack-keystone14:47
*** stevemar has joined #openstack-keystone14:52
*** ChanServ sets mode: +v stevemar14:52
*** browne has joined #openstack-keystone14:57
dolphmdstanek: left some comments on https://review.openstack.org/#/c/77215/ -- the exception handling obviously isn't tested... but i can't imagine what's there is useful today, either?14:58
*** leseb has quit IRC14:59
stevemaris there a reason we haven't approved https://review.openstack.org/#/c/59914/12 (from ML) ?14:59
dolphmstevemar: lack of review focus on the client?14:59
stevemardolphm, ding ding ding15:00
dolphmstevemar: that'd be great to have (and it's calling your code, dstanek)15:00
stevemaryeah, i was wondering if it conflicts with dstanek 's code, but i don't think it does15:00
*** gokrokve_ has quit IRC15:01
*** gokrokve has joined #openstack-keystone15:01
bknudsondolphm: looks like auth_token tries to create the directory if it didn't exist.15:02
dolphmbknudson: but it can't properly detect the condition, afaict. what raises an IOError?15:03
bknudsondolphm: I'd have to try it out.15:04
dstanekdolphm: tempfile does raise IOError in some cases15:04
dstaneki don't know if it's in the code path we are using, but it's there15:04
dolphmbknudson: dstanek: http://pasteraw.com/sonyss4gove3d5n1dvt5ben76obxeq915:04
dolphmbknudson: dstanek: so maybe it should catch (IOError, OSError) ?15:05
bknudsonwhy would IOError be raised?15:05
bknudsonand if it is, should auth_token try to write again after checking the directory exists?15:05
*** leseb has joined #openstack-keystone15:05
dstanekbknudson: there are a few places in the tempfile module that potentially raise it - stat-ing a file, looking for the default tempdir, etc.15:06
*** joesavak has joined #openstack-keystone15:07
bknudsonok. I don't have a problem with catching IOError, OSError.15:08
dolphmbknudson: dstanek: i'll rev the patch as such15:09
dolphmdstanek: bknudson: any reason not to use tempfile.mkstemp instead of tempfile.NamedTemporaryFile?15:15
bknudsonNamedTemporary file closes automatically in context.15:16
dstanekdolphm: the named version has a filename on disk that you can get at - i don't think the other guarantees that15:17
dolphmbknudson: mkstemp doesn't?15:19
dolphmdstanek: http://docs.python.org/2/library/tempfile.html#tempfile.mkstemp the last line in the doc says you get back a file name15:19
bknudsondolphm: mkstemp() returns a tuple containing an OS-level handle to an open file15:19
bknudsonso you'd have to close it.15:19
bknudsonand you'd have to close it even if there was an exception... so would need try/except15:20
dstanekdolphm: not entirely sure why, but i remember named as being safer15:23
dstanekplus what bknudson said15:23
*** chandan_kumar has joined #openstack-keystone15:24
dstanekdolphm: oh, wait; i had to look through the module, but i remember :-)15:25
bknudsonI used to be able to set "disable_service mysql ; enable_service postgresql" in my devstack localrc to run postgres...15:25
dstanekyou want NamedTemporaryFile over TemporaryFile because you are guaranteed a filename on the filesystem15:25
dstanekyou want NamedTemporaryFile over using mkstemp directly because of what bknudson said15:26
dolphmdstanek: ah, interesting15:27
*** david-lyle has joined #openstack-keystone15:28
dstanekstevemar: beat me to the changing passwords review!15:30
*** ayoung-ZZzZzzZz_ is now known as ayoung15:30
ayoungdstanek, you may be my new favorite person.15:31
*** nkinder has joined #openstack-keystone15:32
dstanekayoung: ?15:32
ayoung"David StanekUploaded patch set 80."15:32
dstanekis that a good thing or a bad thing?15:32
dstanekah15:32
ayoungI'll let you male that call15:32
stevemardstanek, you can still review it, too :P15:32
dstaneki didn't address morganfainberg_Z's quesiton about optional dependencies15:32
ayounglooking now15:32
ayoungI likwe 30 * 60....Ah, whateve15:34
dolphmayoung: i'm working on revving the atomic write patch, btw15:34
ayoungdolphm, sounds good.  Nits, or something real?15:35
dolphmayoung: both, i'd say15:35
dolphmayoung: nitty refactor and i think improving exception handling significantly15:35
ayoungAh, that is good15:36
*** bvandenh has quit IRC15:38
dolphmayoung: this is where i'm at http://pasteraw.com/z6plh1drrkrh19v14sbqfr5ya3129f15:38
dolphmayoung: centralizes the exception handling into _atomic_write_to_signing_dir() and includes a catch for OSError15:39
*** bvandenh has joined #openstack-keystone15:41
ayoungdolphm, looks about right.15:42
ayoungdolphm, so, revoke events looks pretty close.  Found a bug last night in v2 token handling.  What I'd like to do is this:15:42
ayoungknock out any last things we find on review and get it merged15:43
ayoungadd in a line in the docs:  this is new and experimental15:43
ayoungengage my own upstream QA to work on tempest tests while I work on a client piece15:43
ayoung\15:44
*** thedodd has joined #openstack-keystone15:44
ayoungFor Juno, we can work to make it the default mechanism and grow morganfainberg_Z 's ephemeral support on top of it15:45
dolphmayoung: sounds like a reasonable goal; i'm trying to knock out a few smaller things, and then i want to spend the rest of the day (as necessary) on revocation15:46
ayoung++15:46
dolphmayoung: granted today is meeting day... bad timing!15:47
ayoungof course.15:47
dolphmbknudson: this is gating https://review.openstack.org/#/c/76949/ - and then we can land your fixes in keystone15:48
bknudsondolphm: I rebased since a migration 41 merged on it.15:49
ayoungdolphm, is the autoinc for sqlalchemy well supported?  morganfainberg_Z suggested I use it in place of uids, and considering the conversation about 10000  speed up, I am tempted to do so15:49
dolphmayoung: yeah you don't have any reason to use UUIDs there15:49
dolphmayoung: well...15:49
ayoungdolphm, my thinking was just to avoid contention for the database15:49
ayoungwe don't select or index on them, but sql does treat it as a pkey15:50
dolphmayoung: yeah... use either15:50
*** devlaps has joined #openstack-keystone15:50
dstanekstevemar: i think i may just fix it...so we can get rolling15:50
ayounggoing to try it with pkey...I'll put something in the migration test to check for mysql and postgres15:50
stevemardstanek, sure thing15:51
dolphmayoung: actually hold up, i'd rather not see any unnecessary flux on the patch15:52
ayoungdolphm, its OK, I can just test it out.  I won't submit it15:52
dstanekstevemar: i tired out your test suggestions and they don't work :-(15:56
dstanek*tried15:56
dstanekstevemar: if you don't set the user_id on client it is set to None15:57
stevemardstanek, hmm, sec15:58
stevemardstanek, could use self.TEST_USER15:59
dstanekstevemar: let me give that a try15:59
stevemarrather than create a new one15:59
dstanekstevemar: the author basically reused this http://git.openstack.org/cgit/openstack/python-keystoneclient/tree/keystoneclient/tests/v2_0/test_users.py#n20915:59
stevemarahh okay15:59
stevemarmy second comment was referring to line 215 of your link16:00
*** devlaps has quit IRC16:11
*** devlaps has joined #openstack-keystone16:11
dstanekstevemar: what is the value of specifying the response body and asserting that we got it? just that it was passed through?16:12
stevemardstanek, i take that back .. i wanted to make sure the manager was returning the right thing16:13
stevemarthe other stuff i don't think is helpful16:13
dstanekstevemar: in this case the manager method won't return anything16:15
dstanekstevemar: or i should say isn't - looking to see if i messed it up or if it was already messed up16:16
stevemarassertIsNone then?16:17
*** bvandenh has quit IRC16:17
dstanekstevemar: it's actually correct; we get an empty body back from the update16:18
dstanekyes, assertIsNone would make it clear that we expect that16:18
stevemarcool cool16:18
stevemari'm good with that16:19
dstanekprobably should be a 204, but it's returning a 200 with no content16:19
stevemaryeah, that's what i was wondering ...16:24
dstanekstevemar: i was just up too late this morning - don't listen to me16:26
dstanekerrr...last night16:27
ayoungdolphm, http://paste.fedoraproject.org/82230/95071113/  is the diff for chaning over to sql.Integer for the sql backend16:32
ayoungincluding a good bit of testing16:33
ayounghalf the patch is a refactor moving insert_dict  into the base class16:33
dstaneki don't see a lot of _() in keystoneclient - do we do something different/special there?16:37
*** harlowja_away is now known as harlowja16:40
*** harlowja has quit IRC16:41
dolphmdstanek: it's just relatively new in the client16:41
dolphmdstanek: (if it's there at all?)16:41
bknudsondstanek: I'm not sure how _() would work in the client library16:41
dstanekdolphm: i don't think it's there at all16:42
bknudsonthey'd need to have the translated strings.16:42
dstanekbknudson: yeah, they would need to be in the package that we release to pypi16:43
bknudsonand you'd need to install the translation domain or something?16:43
dstanekbknudson: i think most things look to the environment to see what to use16:43
bknudsondstanek: keystone does "gettextutils.install('keystone', lazy=True)"16:44
bknudsonso it would also have to gettextutils.install('keystoneclient', lazy=True)" ?16:44
bknudsonor is it keystoneclient would do that with it's _?16:44
bknudsonkeystoneclient would need a different _.16:45
*** gyee has joined #openstack-keystone16:48
dolphmtoken revocation has a +1 from jenkins https://review.openstack.org/#/c/55908/16:48
dstanekbknudson: good question...no idea; i've never really had to worry about dealing with non-English languages16:50
dstanekand when i did it was using Python's gettext directly16:51
*** leseb has quit IRC16:56
*** harlowja has joined #openstack-keystone16:56
*** leseb has joined #openstack-keystone16:57
dstanekstevemar: just pushed an update16:57
stevemardstanek, just reviewed it16:57
dstanekstevemar: gracias16:58
stevemardstanek, de nada16:58
stevemardstanek, if you want to stay in the keystoneclient groove: https://review.openstack.org/#/c/60820/17:07
*** topol has quit IRC17:12
*** rwsu has joined #openstack-keystone17:14
*** stevemar has quit IRC17:21
*** marcoemorais has joined #openstack-keystone17:24
gyeeayoung, trying to understand your comment here https://review.openstack.org/#/c/55908/80/keystone/common/controller.py17:27
gyeeI thought validate_token also hit the caching layer, no?17:28
ayoungself.token_api.token_provider_api.validate_token(  does nt get cached17:29
gyeebut it calls token_api.get_token() right?17:30
ayounggyee, what I would like to do, though, is have validate token be responsible for unpacking the token in the future17:31
ayoungand using the guts of the PKI token that was handed in17:31
ayounggyee, ideally the code will be identical between client and server with the exception of how the revocation events are fetched17:31
gyeeayoung, sure, for PKI token, Keystone is essentially its own consumer17:32
*** leseb has quit IRC17:32
ayoungright, and we want to move to ephemeral in Juno17:32
*** leseb has joined #openstack-keystone17:32
gyee++ for ephemeral, we have problem with stale data in general17:33
*** fabiog has joined #openstack-keystone17:36
dstanekstevemar: i added a couple of comments to that review17:36
*** leseb has quit IRC17:36
*** stevemar has joined #openstack-keystone17:44
*** ChanServ sets mode: +v stevemar17:44
*** jraim_ has joined #openstack-keystone17:45
*** henrynash has joined #openstack-keystone17:48
*** henrynash has quit IRC17:48
*** browne has quit IRC17:50
*** henrynash has joined #openstack-keystone17:51
*** topol has joined #openstack-keystone17:51
*** chandan_kumar has quit IRC17:53
*** jraim has quit IRC17:53
*** jraim_ is now known as jraim17:53
*** topol has quit IRC17:55
*** gokrokve has quit IRC17:55
*** gokrokve has joined #openstack-keystone17:56
ayounggyee, can you make the point on the discussion about the mapping table approach to deconflicting userids17:57
*** browne has joined #openstack-keystone17:57
gyeeayoung, sure17:58
ayoung++17:58
*** topol has joined #openstack-keystone17:59
gyeeayoung, for read-only LDAP deployment, user don't live or managed by Keystone17:59
ayounggyee, yeah.17:59
ayounggyee, same will be true for Federated17:59
ayoungand I don't want stale data in the Identity portion of out table17:59
ayounger17:59
ayoungdatabase18:00
gyeeinfra services don't use the user ID I don't think18:00
*** gokrokve has quit IRC18:00
*** gokrokve has joined #openstack-keystone18:01
gyeefor usage tracking and aggregation, it is whatever ID that Keystone assigns18:01
lbragstadFYI, pushed hacking check for spaces after # in comments: https://review.openstack.org/#/c/77950/2 ( morganfainberg_Z dolphm )18:05
*** morganfainberg_Z is now known as morganfainberg18:08
*** jamielennox|away is now known as jamielennox18:10
*** haneef_ has joined #openstack-keystone18:11
*** haneef_ has quit IRC18:18
*** haneef_ has joined #openstack-keystone18:21
*** gokrokve has quit IRC18:25
*** gokrokve has joined #openstack-keystone18:25
*** henrynash has quit IRC18:29
*** gokrokve has quit IRC18:29
*** leseb has joined #openstack-keystone18:32
*** achampion has quit IRC18:42
*** achampion has joined #openstack-keystone18:49
*** gokrokve has joined #openstack-keystone18:50
*** thedodd has quit IRC18:56
*** thedodd has joined #openstack-keystone19:00
gyeedstanek, but encoding is generally slow, no?19:00
gyeeI mean it adds overhead19:01
dstanekgyee: i haven't measured, but probably a little19:01
dstanekthat won't bypass encoding in Py2 for unicode strings, which need to be encoded19:02
dolphmlbragstad: i love that you patch failed the hacking codebase itself19:03
dolphmyour*19:03
morganfainbergdolphm, hehehe19:03
*** finite has joined #openstack-keystone19:04
dolphmlbragstad: although, looking at the failures, i'm wondering if it should be "whitespace" instead of "a single space"19:04
dolphmlbragstad: i'm not fan of leaving commented out / indented code in the codebase (and if you do, it should be in blockquotes), but the license block has a sort of decent use case for "whitespace", i think19:05
lbragstaddolphm: yeah I was thinking about htat too19:05
dolphmlbragstad: another valid exception :-/ "#!/usr/bin/env python"19:05
lbragstadI would say we'd need to add that with a regex19:05
lbragstadso, we would need to allow the following cases: '#TODO', '#FIXME', '#NOTE', '#!/usr/bin/env python',19:07
dolphmlbragstad: why do you think we should keep #TODO instead of requiring # TODO ?19:07
dolphmlbragstad: is there some special handing for #TODO in IDE's or something?19:07
lbragstadI agree with switching to # TODO, I was just being consistent with the H101 check19:08
lbragstadI would rather have it be #<space>TODO, or NOTE, or FIXME19:08
lbragstadbut, I was looking through the existing checks and though 'I suppose I better not break H10119:09
lbragstad'19:09
lbragstaddolphm: afaict there is no special handling for TODO in IDEs... I use vim19:11
dolphmlbragstad: are there tests or something that require supporting #TODO?19:12
dolphmlbragstad: (that you'd have to change)19:12
*** fabiog has quit IRC19:12
lbragstadchecking19:12
finiteHey folks, I'm trying to set up SSL on my keystone install, and have been going through this: http://docs.openstack.org/developer/keystone/configuration.html#ssl but coming up short. The service doesn't seem to start back up after making the changes. I am using a cert provided by our internal PKI19:13
finiteIs there a log other than the system log I can look at for keystone failing to start?19:14
lbragstaddolphm: not seeing any tests specifically for ensuring '#TODO'19:14
dolphmlbragstad: then kill support for it :)19:15
lbragstad:)19:15
ayoungfinite, turn on debugging:19:16
*** gokrokve_ has joined #openstack-keystone19:17
finiteThanks ayoung, I'll turn on debugging and see what that gleans me.19:18
dstaneklbragstad: ensuring that there is no space?19:19
lbragstadhttps://review.openstack.org/#/c/77950/19:19
lbragstaddstanek: ^19:19
lbragstaddstanek: do you know if that is tested somehow in hacking?19:19
lbragstads/that/the case where #TODO/19:20
lbragstadtest: hacking.tests.test_doctest.HackingTestCase.test_pep8(hacking_todo_format-line-4)19:20
dstaneklbragstad: ha, i have the same check19:20
lbragstad++19:21
*** gokrokve has quit IRC19:21
lbragstaddstanek: yeah, we were talking about it last night when reviewing ayoung 's patch for token revocation, I wanted to go through and fix all the #style comments19:21
stevemaranyone know the pluggable auth structure for client well enough? who is not jamie?19:22
lbragstadand it was mentioned that we should try and add that check to hacking19:22
ayoungstevemar, a bit19:22
dstaneklbragstad: mine is more general i think - http://paste.openstack.org/show/72283/19:22
stevemarayoung, i don't wanna bug you though, you got enough on your plate :)19:22
dstaneklbragstad: i fixed our current master to pass that test19:23
lbragstaddstanek: yep, yours is...19:23
ayoungstevemar, 'salright..I'm mostly waiting for feedback right now19:23
lbragstaddstanek: where does that live?19:23
dstaneklbragstad: how quickly do they accept stuff like that into hacking?19:23
lbragstaddstanek: that I'm not sure...19:23
dstaneklbragstad: right now in a local stash - that's a part of the bunch i'm going to submit by the end of the day19:24
lbragstadit didn't take long to attact a couple -1's after I pushed it for review though19:24
stevemarayoung, okay, so i'm trying to add auth via oauth tokens, but I don't want to go and mangle all the client and httpclient stuff that's already set up19:24
dstanekis hacking an -infra thing?19:24
*** amcrn has quit IRC19:24
lbragstaddstanek: well, i know when new stuff goes in it can break a bunch of stuff... so I would image the infra guys are usually all over is19:24
lbragstads/is/it19:24
ayoungstevemar, I think we still need to figure out how to select the right plugin for the CLI use cases ,but fro calling from pyuthon code...you should be creating a new auth plugin19:25
dstaneklbragstad: that's part of the reason i'm putting my checks in out codebase - some other projects may not be able to upgrade to hacking (with my checks) easily19:26
lbragstaddstanek: I like the way you do the isspace detection better than the way I did mine19:26
lbragstaddstanek: right..19:27
dstaneklbragstad: i'll ping you when i have the review up...i have to chop out the pieces that are not yet working19:27
lbragstaddstanek: I modeled the space detection after the existing way in hacking here: https://github.com/openstack-dev/hacking/blob/master/hacking/core.py#L12519:27
lbragstaddstanek: what can I help you with?19:28
lbragstadI can work on throwing a couple up if you want me to19:28
dstaneklbragstad: that would be great19:28
lbragstaddstanek: sure thing, what do you need?19:28
dstaneklbragstad: i have been using a mix of ast, logical_line and physical line checks based on what i am looking for19:29
dstaneklbragstad: let me get this stuff in order so you can see what i'm doing and then we can talk about the ones on the list that i haven't tackled yet19:29
lbragstaddstanek: cool, that sounds like a plan19:30
dstaneklbragstad: are you comforable with ast?19:30
dstaneklbragstad: i haven't worked on my pet peeve at all: % should not be used in log statements19:30
stevemarayoung, https://review.openstack.org/#/c/77977/ WIP19:31
ayoungstevemar, you missed an important step19:31
ayoungadding jamielennox to the review19:31
ayoung:)19:31
stevemarayoung, i just posted it 2 seconds ago :)19:31
lbragstaddstanek: not entirely,19:32
ayoungstevemar, so...can you work on a test for it, so I can see how you are planning to call it?19:32
stevemarhttps://review.openstack.org/#/c/77977/1/keystoneclient/tests/v3/test_oauth1.py19:32
dstaneklbragstad: the % you can do using logical lines and matching19:33
dstaneklbragstad: ast would be used for more complicated stuff that needs context like try-except-pass19:33
lbragstadso, tighter control on syntax almost19:34
stevemarayoung, the only data in the post request to /auth/tokens should be an empty oauth1 property, and oauth1 in methods. The other oauth data, such as consumer and access keys, are in the request header19:35
*** leseb has quit IRC19:36
dstaneklbragstad: yeah, you are looking the the tree structure of the code instead of just text matching19:36
ayoungstevemar, this is going to be fun  to test, isn't it19:37
lbragstaddstanek: nice19:37
dstaneklbragstad: here is a snipped using AST that looks for people using None in assertEqual - http://paste.openstack.org/show/72284/19:38
stevemarayoung, just do the usual mock request / response test, it should match the expected structure19:38
dstanekgyee: question about your comment on the atomic write patch - why would you need a retry counter? i think tempfile already has that builtin19:40
lbragstaddstanek: makes sense19:40
dstaneklbragstad: excited that i have a partner in crime19:41
gyeedstanek, really?19:41
dstanekgyee: for finding a filename or dirname to use (pretty sure); what case were you thinking of?19:41
lbragstaddstanek: hopefully I'm helpful, I think once I see the review I should be able to get a little more context19:42
gyeedstanek, but the code is essentially doing a retry on IOError or OSError19:42
gyeehence my comment19:42
ayoungstevemar, you are assuming my mind has fully groked the test code in the client...I'm still making the transition....my client work has focused on Auth Token thus far, and while I get what jamielennox is doing, I still need to see it written out in black and white.  Add to that the fact I've not looked at oauth in a long time, and current work has me in a few directions....19:43
*** topol has quit IRC19:43
ayoungwrite a test...it will be required for commit, and it will make it easier for all to review19:43
*** joesavak has quit IRC19:43
finiteHey @ayoung, turning on debugging I see this in they keystone.log: 2014-03-04 19:23:42.625 8461 TRACE keystone.common.environment.eventlet_server SSLError: [Errno 336445442] _ssl.c:355: error:140DC002:SSL routines:SSL_CTX_use_certificate_chain_file:system lib19:44
ayoungfinite, are your certs readable?19:44
*** stevemar2 has joined #openstack-keystone19:44
*** ChanServ sets mode: +v stevemar219:44
finite644 root.root19:46
*** wchrisj has quit IRC19:47
*** arborism has joined #openstack-keystone19:47
*** arborism is now known as amcrn19:47
finiteit looked like glance had an issue like this at one time? https://bugs.launchpad.net/glance/+bug/116052919:47
*** stevemar has quit IRC19:48
*** wchrisj has joined #openstack-keystone19:49
*** stevemar has joined #openstack-keystone19:49
*** ChanServ sets mode: +v stevemar19:49
lbragstaddolphm: looks like we are going to have to throw https://review.openstack.org/#/c/77950/ in https://github.com/jcrocholl/pep8/blob/master/pep8.py19:49
stevemarayoung, i hear ya19:50
*** stevemar2 has quit IRC19:51
lbragstadcc dstanek ^19:52
finiteayoung, I found one that wasn't readable. Looks like that might have fixed it. Thanks for helping!19:53
stevemardstanek, qq19:53
ayoungfinite, so...want a better approach?19:54
stevemardstanek, i think there was a typo in your first comment: https://review.openstack.org/#/c/60820/12/keystoneclient/tests/v3/test_oauth1.py19:54
*** topol has joined #openstack-keystone19:54
finiteFor sure19:54
finiteayoung, always heh19:56
ayoungfinite, certmonger19:56
ayoungfinite, I'm messing around with it right now19:56
ayoungbut using certmonger to talk to a CA19:56
ayoungor even just to track your "selfsigned" if you don';t have a CA19:57
ayoungwhat platform are you on?19:57
finiteCentOS 6.4 64bit19:57
finiteayoung I'll dig into that, thanks. :)19:58
ayoungfinite, are you going to be getting real CA certs, or just doing selfsigned?19:59
finiteCompany signed PKI, internal.19:59
ayoungfinite, ah...what CA are you using?19:59
ayoungsoftware20:00
*** leseb has joined #openstack-keystone20:01
finiteayoung, no idea, other it folks generate everything and I can't find any info in the cert20:02
ayoungfinite, ask them...one nice thing about certmonger is it will do renewals for you20:02
*** gyee has quit IRC20:03
ayoungI work with one of the certmonger core devs..one issue we have is there are not enough people requesting support for their CAs, so we don;t know whom to support20:03
finiteayoung, nice I'll hit them up as I have a few more to request. Thanks for all your help, greatly appreciated20:03
ayoungDogtag and FreeIPA are well supported20:03
ayoungas is selfsigned20:03
*** krsna has joined #openstack-keystone20:08
krsnaI have a +2 already and a few +1s. I just need an aproved https://review.openstack.org/#/c/77294/ if anyone cares to take a look ;)20:09
stevemardstanek, i think i addressed your comments, through either reply backs, or new code: https://review.openstack.org/#/c/60820/20:09
morganfainbergbknudson, https://review.openstack.org/#/c/77404/ strings fixed, added a bunch of punctuation20:16
bknudsona bunch!!!!20:16
morganfainbergyeah, now all our helpstrings end with proper punctuation20:16
morganfainbergshould have made them all ! or even better ?20:17
morganfainbergreally confuse people20:17
*** henrynash has joined #openstack-keystone20:19
ayoungdolphm, so...zuul is getting slow again.  I don't plan on posting another revision of the revoke patch20:21
ayoungI think that what we have (assuming it passes check and would pass gate) is what we are going to get20:21
dolphmayoung: ack, that's what i'm hoping20:26
dolphmayoung: i'm reviewing now until the next openstack meeting20:26
dolphmayoung: TC is meeting about barbican right now though20:26
dolphmif anyone has free review bandwidth: https://review.openstack.org/#/c/75727/20:29
dolphmbknudson: the second patchset in that sequence failed jenkins ^20:29
lbragstadsure, I was going to follow up on that one, and I'm waiting to give dstanek a hand20:30
bknudsondolphm: I'll change it to address topol's comment20:31
dolphmbknudson: rechecked (bug 1284371)20:31
dolphmayoung: i thought you weren't including the extension by default?20:32
ayoungdolphm, its in the paste but not in the pipeline20:33
dolphmayoung: it's in the pipeline20:33
ayoung?20:33
ayounghttps://review.openstack.org/#/c/55908/82/etc/keystone-paste.ini20:35
ayoungonly change to the paste is to create the filter20:36
dolphmayoung: wtf, maybe i was reviewing an old patchset20:36
dolphmayoung: i went back to get a link and it was gone20:36
dolphmayoung: yep... i have draft comments on patchset 7720:36
* dolphm starts over20:36
*** topol has quit IRC20:36
ayoungdolphm, post them as is and I will look20:36
dolphmayoung: no bother, i had only made it through a few files20:36
ayoungk20:36
dolphmdid that object?.maybe_call_me() syntax ever land in java?20:37
dolphmdstanek: lbragstad: https://review.openstack.org/#/c/77950/ got a -2 ... not sure i understand20:43
*** finite has quit IRC20:43
lbragstaddolphm: I think what Joe means here is that since '#  This is a comment' is a pep8 style "guideline" it should live in the pep8tool20:44
lbragstadand not be carried in the openstack-dev/hacking project20:44
lbragstadwhen it could be made upstream20:44
*** krsna has quit IRC20:45
*** derek_c has joined #openstack-keystone20:47
*** derek_c has quit IRC20:48
*** derek_c has joined #openstack-keystone20:48
ayoungbknudson, so we are just going to add enabled into the endpoint, right? We won't be changing the API to not return disabled endpoints?20:49
bknudsonayoung: the fix to return only enabled endpoints in the catalog is https://review.openstack.org/#/c/77441/20:50
ayoungbknudson, only the SQL backend?20:50
ayoungah..in the earlier patch20:51
bknudsonayoung: I looked at the other backends and those ones didn't make sense to have disabled endpoints20:51
ayoungyeah, saw the comment20:51
bknudsone.g., with the templated one... you just wouldn't put the endpoint there if it was disabled.20:51
bknudsonayoung: so the thing that's missing is disabled services.20:52
ayoungthat is OK.20:52
bknudsonI wanted to get some feedback on the way that I'm doing endpoints20:52
ayoungI would almost argue that a service should not  be disabled20:52
ayoungjust endpoints20:52
bknudsonthe changes to handle services are essentially the same.20:52
ayoungbknudson, so, say an endpoint is disabled, what would the workflow be like to reenable it20:57
dstanekdolphm, lbragstad: that's silly; i have this check it my current batch20:57
dstanekdolphm, lbragstad: we can decide where it goes from there20:57
ayounglist endpoints will still show it, it just won't show up in the catalog?20:58
lbragstaddstanek: dolphm looks like we will have to carry this in Keystone then?20:58
bknudsonayoung: you can enable an endpoint with { 'endpoint': { 'enabled': True } }20:58
ayoungso list the endpoiinbt and update20:58
dolphmlbragstad: i'm okay with that20:58
ayoungyeah, I meant finding it20:58
bknudsonayoung: yes, it still shows up in the endpoints list.20:58
ayoungcool20:58
bknudsonit's only the token catalog is affected.20:58
bknudsonso if you validate your token maybe you'll get a different catalog!20:58
lbragstadnova has a bunch of their own check s20:59
dstaneklbragstad: maybe; i was putting in keystone because i don't understand what they want20:59
ayoungshould  endpoint = self.get_endpoint(entry.endpoint_id)  be  endpoint = self.get_endpoint(entry.endpoint_id, enabled=True)  in the future?20:59
lbragstadthey == openstack-dev/hacking or pep8tool?20:59
ayoungnah, nbevermind20:59
bknudsonayoung: is this in the contrib OS-EP-FILTER?21:00
ayoungI was thinking in the list itself21:00
ayoungbknudson, yeah21:00
dstaneklbragstad: hacking21:00
ayoungget_endpoints...21:00
bknudsonayoung: I didn't touch it too much since I wasn't really sure what OS-EP-FILTER was doing...21:00
ayoungbknudson, no, what you did looks right to me21:00
bknudsonespecially when it turned out that none of that code was tested.21:01
ayoungI was just thinking about disable in general, as we have that for many entities now21:01
bknudsonit would be interesting to see if other entities store enabled in the extras column.21:01
*** stevemar has quit IRC21:02
ayoungbknudson, that whole patch series has 2 +2s on it21:02
ayoungbknudson, we've pulled a few of them out21:02
ayoungproject and domain are not21:03
ayoungbut I thik that project was origianlly (tenant)21:03
ayoungbknudson, domain, projet an d user all have explicit enabled fields, I just checked21:05
bknudsonso not service21:05
ayoungyeha, neither of the service catalog entities in master21:05
bknudson/v3/services says enabled (boolean) , so I assume it can actually be anything.21:06
lbragstaddstanek: do you have a check for vim headers?21:06
bknudsonand disabling a service has no effect.21:06
dstaneklbragstad: nope21:06
dstaneklbragstad: that would be a great addition though21:07
lbragstaddstanek: ok, yeah I think so too21:07
bknudsonayoung: I think services is the only one that has enabled in extras now.21:08
ayoungJuno for that.21:08
ayoungwe are going to be doing a bit with the service catalog, to include endpoint binding of tokens and service/endpoint specific roles21:09
ayoungRBAC and Catalog should be one summit track21:09
dolphmbknudson: ++21:16
*** marekd is now known as marekd|away21:20
derek_cwhen running openstackclient, by that I mean when typing "openstack" in terminal, I get a "ERROR: openstackclient.shell Exception raised: Authorization failed: The resource could not be found. (HTTP 404)".  Does anyone have a clue what's going on?21:23
derek_cI suspect it has to do with me setting OS_AUTH_URL to a v2.0 API?21:23
derek_cI mean URL21:23
bknudsonderek_c: there's also an option / env var for the identity api version.21:24
derek_cbknudson: I have set OS_IDENTITY_API_VERSION to 321:25
bknudsonderek_c: then you'll need to have the OS_AUTH_URL set to a v3 endpoint.21:25
derek_cbknudson: how do I find out the correct endpoint?21:25
derek_cI tried /v3.0 and /v321:26
bknudsonderek_c: do a GET on the base URL and the result should include the versioned endpoints.21:26
lbragstaddstanek: I assume the Keystone specific hacking stuff is going to live in /keystone/hacking/ , right21:28
lbragstad?21:28
derek_cbknudson: ah, it worked!21:28
derek_cbknudson: thanks a lot :)21:28
bknudsonderek_c: no problem.21:28
morganfainberglbragstad, keystone.hacking? keystone.tests.hacking?21:32
*** stevemar has joined #openstack-keystone21:34
*** ChanServ sets mode: +v stevemar21:34
morganfainbergbknudson, dolphm, ayoung, ok so infra consensus, no periodic job to update sample.conf, moving to a non-voting check job that will say "sample config is out of date"21:38
ayoungmorganfainberg, meh21:39
morganfainbergdolphm, bknudson, ayoung, next step will be to make it part of doc build and release building and evict it from git21:39
morganfainbergno pep8 checks21:39
lbragstadmorganfainberg: dstanek and I are looking to add hacking checks21:39
morganfainberglbragstad, nod, i was offering other options21:39
lbragstadmorganfainberg: ahh, right21:40
morganfainbergso sample config will never exist in the git tree, will be always built.21:40
bknudsonmorganfainberg: I think that will work better.21:40
morganfainbergayoung, it's the direction the infra team wants to go. and if we can get there it solves the same one.  #1 goal - no pep8,21:40
dstanekmorganfainberg: it'll be in keystone.hacking21:40
bknudsonI'm worried about the config file changes when the oslo libraries are updated.21:40
morganfainbergi would prefer it being in git tree, but that is personal preference.21:40
dolphmmorganfainberg: hmm, what was the reasoning against a periodic job?21:40
morganfainbergdolphm, we control the artifacts. it can be built at install time21:41
dolphmmorganfainberg: wait, no sample config in tree?21:41
ayoung++21:41
morganfainbergdolphm, make it something we build just like docs...it really is a doc21:41
morganfainbergdolphm, yep, no sample config in the git tree, it would be built like docs are built and at package/setup time21:41
dolphmmorganfainberg: i want it to be published, and i want it to be easy to copy it and modify it to my liking; where is it going to be published?21:42
morganfainbergdolphm, longer term, with the documents21:42
morganfainbergdolphm, published with each merge and each release21:43
morganfainbergi would keep the generator in tox21:43
morganfainbergdolphm, i don't see why a specific project can't keep it in tree as well21:43
*** arunkant has quit IRC21:43
morganfainbergbut it wont be gated upon being "up to date"21:43
morganfainbergcan't do that reasonably with oslo libraries (~20 or so targeted for juno)21:44
bknudsonmaybe oslo libraries will change how they do config21:44
morganfainbergbut initially (today) we will get a non-voting check job that will tell us "hey this is not up to date"21:45
morganfainbergbknudson, ++ i would like that.21:45
morganfainbergbknudson, but dunno how likely that will be21:45
bknudsonmorganfainberg: we just need to push the changes.21:45
morganfainbergbknudson, yeah, there are some minor ones i want to see first, but not sure what they should be doing for config21:46
dolphmmorganfainberg: alright i'm confused now... if the sample conf is not in-tree, then what's not up to date?21:46
morganfainbergdolphm, initially it will be in tree21:46
morganfainbergdolphm, then we will move it to docs and out of tree at that point21:46
dolphmmorganfainberg: openstack-manuals? or keystone/docs ?21:46
morganfainbergdolphm, openstack-manuals21:46
morganfainbergdolphm, and packaging time would stick it in keystone/etc21:47
morganfainbergso it's there for deployment21:47
*** achampion has quit IRC21:47
morganfainbergdolphm, 3 phases.  1: check job that says "Hey in-tree is out of date", 2: openstack-manuals publishing and setup/package time building, 3: evict check-job and git (if project desires)21:48
morganfainbergthe pep8 check would go away in the first phase21:49
dolphmmorganfainberg: alright, so 1) will fail when i change a config opt, alerting reviewers to the fact that i should include a revised sample conf with my patch?21:49
morganfainbergdolphm, correct.  it wont be voting, but it will show in the check run from jenkins that says it's out of date21:50
morganfainbergjust like a py33 fail. sure it failes but it wouldn't block the review21:50
dolphmbknudson: speaking of, check_uptodate.sh failed on https://jenkins06.openstack.org/job/gate-keystone-pep8/445/console21:51
bknudson"kombu_reconnect_delay=1.0"21:52
dolphmbknudson: yep..21:52
bknudsonwtf!21:52
dolphmmorganfainberg: alright i'm in favor.21:52
bknudsonthat was quick21:52
*** topol has joined #openstack-keystone21:54
bknudsonI'll see if I can recreate the pep8 failure.21:54
bknudsonI would expect this to cause everything to fail21:56
bknudsonso... how about disabling the pep8 sample config check?21:56
bknudsonwas able to recreate with a tox -r -e pep821:57
*** gyee has joined #openstack-keystone21:58
ayoungmorganfainberg, dstanek can you guys give at least a +1 to https://review.openstack.org/#/c/55908/  It feels unloved21:58
bknudsonhttps://review.openstack.org/#/c/78024/ -- updated sample config file22:00
ayoungbknudson, +222:01
*** marcoemorais has quit IRC22:02
*** marcoemorais has joined #openstack-keystone22:02
morganfainbergayoung, 82!22:04
ayoungSo long as it doesn't go to 86.  There is no return from 86.  Don't even try.22:05
ayoungdolphm, can we squeeze this one in:  https://review.openstack.org/#/c/47441/22:06
ayoungits a nice performace fix...could go in to RC22:07
dolphmayoung: commit message sounds great - but i defer to bknudson22:08
ayoungsounds good...22:08
*** krsna has joined #openstack-keystone22:08
morganfainbergayoung, and if it hits 88, we're uhm going back in time22:09
ayoungWe'll actually commit patch 57?22:10
morganfainberg5522:10
dolphm82 gets denser and denser as you go22:10
dolphmi'm like 45% through, all nits so far, making coffee22:11
dolphmthere's also dead code in the manager22:11
*** achampion has joined #openstack-keystone22:11
*** marcoemorais has quit IRC22:15
stevemarmorganfainberg, poke22:16
morganfainbergstevemar, ouch!22:16
morganfainbergstevemar, my eye22:16
stevemarmorganfainberg, i'm forwarding a poke request: https://review.openstack.org/#/c/77294/22:16
morganfainbergi saw openstack-dev :P22:17
stevemar:)22:17
ayoungdolphm, I'll be explaining this code to people for years, I am afraid22:18
morganfainbergayoung, nah, likely dolph steve and i will explain it wrong... then you'll correct us22:19
morganfainbergok ok, stevemar  and i will explain it wrong22:19
* stevemar slowly walks away 22:20
morganfainbergstevemar, playing the hulk theme...sadly walking away?22:20
morganfainbergstevemar, http://www.youtube.com/watch?v=uq0HTOGtfSw22:21
stevemarmorganfainberg, as long as you're the one explaining revocation, you can play whatever theme you want22:21
morganfainbergstevemar, i can probably explain it now actually22:21
morganfainbergstevemar, i think some general code restructuring in Juno will make it easier to follow, but it's not that bad22:22
morganfainbergstevemar, it's just a massive code dump, which makes it hard to review22:22
stevemarmorganfainberg, i can get through most of the stuff in contrib, but after that it gets a bit dicey22:22
morganfainbergstevemar, i've been lurking in revocations, tokens, etc for a while now...and changing assignment etc.22:23
morganfainbergi think...22:23
morganfainbergit's warped my brain22:23
ayoungstevemar, something happens, and we immediately tell the contrib code "revoke tokens that have this property"  or "these couple properties"22:25
ayoungthen, when a new token request comes in,  check it against those rules to see if it revoked22:25
ayoungIdeally , we would do all of the revocations based on events, but adding in the new ones right now is just to invasive22:26
dstanekayoung: i'll walk through it again now22:26
dstanekayoung: did you ever do a test to load a few thousand revoked tokens in memory?22:27
ayoungthe part that is trickiest is the "revoke by tree"22:27
ayoungdstanek, nope...22:27
ayoungdstanek, I'd need to somehow keep them from all being related22:27
ayounglike 10K different users each with one22:27
ayoungor a handful22:28
ayoungdef something for tempes22:28
ayoungt22:28
*** devlaps1 has joined #openstack-keystone22:28
*** devlaps has quit IRC22:29
*** leseb has quit IRC22:32
* dolphm coffee took forever but is a magnificent success; attempting to apply extraneous success to token revocation22:33
bknudsonhttps://review.openstack.org/#/c/78030/ -- based on change in nova https://review.openstack.org/#/c/78028/22:35
*** achampion has quit IRC22:39
dolphmbknudson: +222:43
dolphmmorganfainberg: ^22:43
*** lbragstad has quit IRC22:43
dolphmayoung: ^22:43
dolphmstevemar: where is the oauth manager instantiated?22:44
stevemarin the dependency stuff, 1 sec22:44
ayoung+222:48
stevemardolphm, https://github.com/openstack/keystone/blob/master/keystone/contrib/oauth1/core.py#L14422:49
dolphmstevemar: but what instantiates an instance of that class?22:50
stevemardolphm, magic at the dependency layer22:51
dolphmstevemar: something calls oauth1.Manager() so that it's configured and made available...22:51
stevemardolphm, https://github.com/openstack/keystone/blob/master/keystone/common/dependency.py#L19022:52
dolphmstevemar: it's not done where other managers are done to avoid creating a dep on oauth1lib22:52
stevemarcorrect22:52
*** dims has quit IRC22:52
morganfainbergdolphm, +222:54
stevemardolphm, https://review.openstack.org/#/c/64349/22:55
dolphmstevemar: hrm...22:56
dolphmstevemar: revoke should ideally do... whatever that's doing.. too.22:56
dolphmstevemar: and yeah, that's magic to me22:56
stevemarbknudson might be able to shed more light on the magic22:56
stevemardolphm, ^22:56
bknudsonsomething has to call oauth1.Manager()22:57
bknudsonstevemar: when that happens then the instance will be added to the dependency map.22:57
bknudsonand it'll be injected into any other objects that require it.22:57
bknudsonkeystone/auth/plugins/oauth1.py:        self.oauth_api = oauth1.Manager()22:58
bknudsonkeystone/contrib/oauth1/routers.py:        oauth1.Manager()22:58
bknudsonthat's probably where it comes from.22:59
bknudsonif you add the middleware to the paste pipeline it'll eventually create the router.22:59
bknudsonthe constructor really shouldn't be called multiple times...22:59
bknudsone.g., why keystone/contrib/oauth1/validator.py:        self.oauth_api = oauth1.Manager() ?22:59
dolphmbknudson: i'm now wondering if that's a necessary bug ^23:01
dolphmthe "magic"23:01
bknudsondolphm: if it is then there should be a comment.23:02
bknudsonI assume there's no need to an oauthvalidator if the routes aren't there.23:02
dolphmbknudson: i bet if you delete that then you can't spin up oauth23:03
dolphm(outside of tests)23:03
bknudsonI have to run to bowling.23:03
*** marcoemorais has joined #openstack-keystone23:04
*** arborism has joined #openstack-keystone23:05
*** amcrn has quit IRC23:06
*** dims has joined #openstack-keystone23:08
*** marcoemorais has quit IRC23:09
dolphmayoung: +1 for the extra assertions in test_v3_auth -- first time i've seen them23:09
ayoungdolphm, need more.23:11
dolphmayoung: they're waaaay stronger than before though23:11
*** dstanek has quit IRC23:12
dolphmayoung: more importantly, i could draw up what the response looks like for the most part, although i'd have to guess at the datetime format, etc23:12
*** marcoemorais has joined #openstack-keystone23:12
dolphmayoung: on the 'disable a domain' test for example, i'd like to see the domain re-enabled, ensuring that the event still exists, and then re-disabling the domain should revise the event23:13
ayoungdolphm, so...the event only handles revoking things that happen in the past23:15
ayoungnow, tests happen artificially fast23:15
dolphmayoung: you can control time with mock + timeutils23:16
ayoungbut,  re-enable means you get viable tokens again, and the revoke event should not match23:16
ayoungand then  a second disable should just update the time,23:16
dolphmayoung: exactly - i don't see that tested (?)23:16
ayoungso, two events, but still just one path through the tree, and it will catch all of the the events23:16
dolphmor i just haven't gotten to it yet23:16
ayoungnot on domain...its more in testing the tree structure itself23:17
ayoungbut...yeah, more testing better23:17
ayoungbut " domain re-enabled, ensuring that the event still exists"  you mean "renable and new tokens should get through but old tokens should still be revoked"23:18
ayoungthat is a good test23:18
dolphmayoung: correct23:18
ayoungdolphm, can we do that as a follow on?    The more I think about this, the more testing I'd like to do across the board23:19
dolphmayoung: yeah23:20
dolphmayoung: if you're confident it would pass if we wrote it23:20
ayoungdolphm, it is accounted for in the design and tested in other ways23:23
dolphmayoung: pop quiz what does revoke_api.check_token() do if the token is invalid?23:23
ayoungdolphm, it would probably 50023:24
dolphmayoung: i mean as in revoked23:25
ayoungit takes a map, and assumes the values in there have been popleated23:25
dolphmayoung: i don't care about the impl, what's the interface?23:25
ayoungdolphm, if a token is revoked, that means that there is an entry wiuth user and expires at that will match the token, and it will not pass23:25
ayoungspecifically23:25
ayoungmodel is_revoke returns true23:26
ayoungthen in core23:26
ayounghttps://review.openstack.org/#/c/55908/82/keystone/contrib/revoke/core.py  rasises TokenNotFound23:26
dolphmayoung: okay you killed me i'm going to go make dinner23:26
dolphmayoung: https://review.openstack.org/#/c/55908/23:27
ayoungthanks23:27
*** david-lyle has quit IRC23:34
morganfainbergdolphm, ayoung https://review.openstack.org/#/c/78030/ pre-emptively approving23:38
morganfainbergdolphm, ayoung, since we know we want it and it should start gating asap23:38
*** thedodd has quit IRC23:39
stevemarayoung, it's lookin good for you23:53

Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!