17:03:17 <lbragstad> #startmeeting keystone-office-hours 17:03:18 <openstack> Meeting started Tue Aug 21 17:03:17 2018 UTC and is due to finish in 60 minutes. The chair is lbragstad. Information about MeetBot at http://wiki.debian.org/MeetBot. 17:03:19 <openstack> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 17:03:21 <openstack> The meeting name has been set to 'keystone_office_hours' 17:03:58 <kmalloc> ayoung: responded to that email re: ksm and flask apps without paste 17:04:07 <ayoung> TYVM 17:04:31 <kmalloc> ayoung: it is a little wonky becuase yo have to do hoops to load the middleware correctly, but keystone.server.flask.core has code that does exactly what was being asked for. 17:04:41 <ayoung> looking 17:04:50 <kmalloc> https://github.com/openstack/keystone/blob/c896f911efecfddf07a3d1117ced7fc271ee70cb/keystone/server/flask/core.py#L125 17:06:33 <ayoung> kmalloc, I don't think they are doing Stevedore. Is that a requirement? 17:06:41 <kmalloc> nope 17:07:02 <kmalloc> but we use it to load since we only really have entry points we can lean on in some cases. 17:07:16 <kmalloc> stevedore makes it a little easer in some ways, harder in others. 17:07:16 * lbragstad goes to make coffee 17:07:27 <ayoung> so in their code, they explicitly load KSM and add it to the pipeline 17:08:02 <ayoung> ksm = keystonemiddleware.auth.something 17:08:03 <kmalloc> yeah, in the flask way they'd load (import) ksm and then do flask_app.wsgi_app = ksm_init_func_name(flask_app.wsgi_app) 17:08:26 <openstackgerrit> 98k proposed openstack/keystone master: import zuul job settings from project-config https://review.openstack.org/594440 17:08:28 <openstackgerrit> 98k proposed openstack/keystone master: switch documentation job to new PTI https://review.openstack.org/594441 17:08:29 <openstackgerrit> 98k proposed openstack/keystone master: add python 3.6 unit test job https://review.openstack.org/594442 17:08:30 <openstackgerrit> 98k proposed openstack/keystone master: import zuul job settings from project-config https://review.openstack.org/594443 17:08:32 <openstackgerrit> 98k proposed openstack/keystone master: switch documentation job to new PTI https://review.openstack.org/594444 17:08:38 <openstackgerrit> 98k proposed openstack/keystoneauth master: import zuul job settings from project-config https://review.openstack.org/594445 17:08:40 <openstackgerrit> 98k proposed openstack/keystoneauth master: switch documentation job to new PTI https://review.openstack.org/594446 17:08:41 <openstackgerrit> 98k proposed openstack/keystoneauth master: add python 3.6 unit test job https://review.openstack.org/594447 17:08:42 <openstackgerrit> 98k proposed openstack/keystoneauth master: add lib-forward-testing-python3 test job https://review.openstack.org/594448 17:08:43 <openstackgerrit> 98k proposed openstack/keystoneauth master: import zuul job settings from project-config https://review.openstack.org/594449 17:08:44 <openstackgerrit> 98k proposed openstack/keystoneauth master: switch documentation job to new PTI https://review.openstack.org/594450 17:08:52 <openstackgerrit> 98k proposed openstack/keystonemiddleware master: import zuul job settings from project-config https://review.openstack.org/594451 17:08:53 <openstackgerrit> 98k proposed openstack/keystonemiddleware master: switch documentation job to new PTI https://review.openstack.org/594452 17:08:54 <openstackgerrit> 98k proposed openstack/keystonemiddleware master: add python 3.6 unit test job https://review.openstack.org/594453 17:08:56 <openstackgerrit> 98k proposed openstack/keystonemiddleware master: add lib-forward-testing-python3 test job https://review.openstack.org/594454 17:08:57 <openstackgerrit> 98k proposed openstack/keystonemiddleware master: import zuul job settings from project-config https://review.openstack.org/594455 17:08:58 <openstackgerrit> 98k proposed openstack/keystonemiddleware master: switch documentation job to new PTI https://review.openstack.org/594456 17:09:04 <openstackgerrit> 98k proposed openstack/keystone-specs master: import zuul job settings from project-config https://review.openstack.org/594457 17:09:10 <openstackgerrit> 98k proposed openstack/keystone-tempest-plugin master: import zuul job settings from project-config https://review.openstack.org/594458 17:09:15 <openstackgerrit> 98k proposed openstack/ldappool master: import zuul job settings from project-config https://review.openstack.org/594459 17:09:17 <openstackgerrit> 98k proposed openstack/ldappool master: add python 3.6 unit test job https://review.openstack.org/594460 17:09:22 <ayoung> ksm_factory = keystonemiddleware.auth_token.app_factory 17:09:22 <kmalloc> omg. 17:09:23 <openstackgerrit> 98k proposed openstack/pycadf master: import zuul job settings from project-config https://review.openstack.org/594461 17:09:24 <openstackgerrit> 98k proposed openstack/pycadf master: switch documentation job to new PTI https://review.openstack.org/594462 17:09:26 <openstackgerrit> 98k proposed openstack/pycadf master: add python 3.6 unit test job https://review.openstack.org/594463 17:09:27 <openstackgerrit> 98k proposed openstack/pycadf master: add lib-forward-testing-python3 test job https://review.openstack.org/594464 17:09:28 <openstackgerrit> 98k proposed openstack/pycadf master: import zuul job settings from project-config https://review.openstack.org/594465 17:09:29 <openstackgerrit> 98k proposed openstack/pycadf master: switch documentation job to new PTI https://review.openstack.org/594466 17:09:36 <openstackgerrit> 98k proposed openstack/python-keystoneclient master: import zuul job settings from project-config https://review.openstack.org/594467 17:09:37 <openstackgerrit> 98k proposed openstack/python-keystoneclient master: switch documentation job to new PTI https://review.openstack.org/594468 17:09:39 <openstackgerrit> 98k proposed openstack/python-keystoneclient master: add python 3.6 unit test job https://review.openstack.org/594469 17:09:40 <openstackgerrit> 98k proposed openstack/python-keystoneclient master: add lib-forward-testing-python3 test job https://review.openstack.org/594470 17:09:41 <openstackgerrit> 98k proposed openstack/python-keystoneclient master: import zuul job settings from project-config https://review.openstack.org/594471 17:09:42 <openstackgerrit> 98k proposed openstack/python-keystoneclient master: switch documentation job to new PTI https://review.openstack.org/594472 17:09:43 * kmalloc wants to kick openstackgerrit all of a sudden 17:11:43 <kmalloc> lbragstad: are we moving to py3.6 ^ just because... 17:12:06 <kmalloc> lbragstad: i'm not opposed to it, just want to know if it's really a project plan 17:12:40 <kmalloc> ayoung: there is some magic config stuff that has to be passed around with the factory methods 17:12:52 <kmalloc> ayoung: but you can also directly call init, if you're directly importing 17:13:00 <ayoung> kmalloc, I'd like to give them a more workable example 17:13:19 <ayoung> let me see if I have their code. might be easier to discuss 17:13:24 <kmalloc> unfortunately, that is about as workable as you get with webob-middleware wrapping flask 17:13:31 <kmalloc> i looked in depth into this 17:13:39 <kmalloc> ksm is very webob oriented 17:13:52 <kmalloc> so you need to webob-based things. 17:14:16 <kmalloc> it passes down the request and all that stuff just fine, and flask turns it into a werkzeug request a-ok 17:14:19 <ayoung> kmalloc, I just mean something they can actually code. I don't think we've given enough info yet 17:14:54 <kmalloc> sure. i was largely assuming they can derive how it works from our code, but if they need more detail, i can help supply 17:15:18 <kmalloc> i am not in the business of assuming someone can't read the code and either a) determine they have enough info, or b) ask more questions 17:15:24 <kmalloc> i offered to help further if needed. 17:15:32 <kmalloc> (for that reason) 17:50:29 <lbragstad> what just happened? 17:51:07 <kmalloc> lbragstad: someone proposed a TON of "py3.6" stuff 17:52:34 <gagehugo> oh wow 18:03:15 <mtreinish> kmalloc: I think that's all generated by a script. dhellmann has been talking about it on the ML 18:10:45 <lbragstad> that reminds me - i need to get to that thread 18:13:39 <kmalloc> mtreinish: its spaaaaaaaaaaaamy :) 18:14:30 <lbragstad> ok - qq 18:14:49 <lbragstad> i'm trying to figure out the best way to implements DRY tests for https://git.openstack.org/cgit/openstack/keystone/tree/keystone/common/policies/credential.py#n21 18:15:02 <lbragstad> and this is the skeleton i have so far http://paste.openstack.org/show/728547/ 18:18:00 <lbragstad> does anyone have a better idea for testing this stuff? 18:18:49 <lbragstad> i'll settle with http://paste.openstack.org/show/728547/ for now since it's arguable better organized that what we have in test_v3_protection, but open to suggestions if anyone has them 18:24:57 <kmalloc> lbragstad: hm 18:25:11 <lbragstad> feels like i'm duplicating a lot still 18:25:31 <lbragstad> and with how many of these we're going to have to write, i'd like to start off on a better foot if possible 18:25:52 <kmalloc> right, so you are going to duplicate a lot 18:25:58 <kmalloc> it's the nature of testing. 18:26:08 <kmalloc> you can combine *some* things 18:26:18 <kmalloc> in external functions/methods that do the work that is common 18:26:34 <kmalloc> but testing *is* a lot less DRY than the rest of python 18:26:35 <kmalloc> code 18:29:11 <kmalloc> lbragstad: the key is that testing really is going to duplicate work because each test really is isolated for the most part. 18:30:00 <kmalloc> e.g. "get_me_a_token" type method is good to abstract out, "do a request that affects credentials in X way" probably shouldn't be abstracted out 18:30:20 <kmalloc> the workflow of "after setup", should be explicit imo 18:35:55 <lbragstad> maybe something like this? http://paste.openstack.org/show/728550/ 18:39:18 <lbragstad> actually - http://paste.openstack.org/show/728552/ 18:39:19 <kmalloc> lbragstad: that doesn't look too bad, however, i highly recommend not doing "mixin" style objects. It makes the analysis (static and otherwise) harder. 18:39:38 <kmalloc> lbragstad: so, imo, don't have objects not based upon the unit test base. 18:40:16 <lbragstad> you mean have all test classes inherit from the test class directly instead of `object`? 18:40:20 <kmalloc> yeah 18:40:40 <kmalloc> if it's a common "case" make the common case on the module base test. 18:41:37 <kmalloc> i really really hate the whole "this object implements tests and is just mixed in to make it work" 18:42:04 <kmalloc> i would rather see fully featured test cases. 18:42:39 <kmalloc> e.g.: TestCase->CredentialBaseTestCase[common code/common tests]->OverrideTestCaseClass[overrides/specifics] 18:43:06 <kmalloc> but thats just me. 18:43:32 <lbragstad> yeah - i can see the point 18:43:54 <kmalloc> remember special cases such as (iirc) _ prefixed tests aren't run. 18:44:16 <lbragstad> yeah - test's have to start with `test` 18:44:28 <lbragstad> otherwise i don't think they're picked up by discovery 18:44:50 <kmalloc> if we are "mixing" in tests because we're doing DRY, i think we're just doing it wrong, it means we are over-doing the DRY bit. 18:45:10 <kmalloc> i would be 100% ok with tests being all explicit no shared code, because it is explicit 18:45:20 <kmalloc> it doesn't allow "accidently" changing one test and affecting others 18:45:42 <kmalloc> there are cases where it has been painful to fix things because tests are so inter-mingled 18:46:09 <kmalloc> tests are a case i would highly advocate for not DRYing *except* to make adding new tests easy. but any/all code flow for a test should be explicit. 19:31:32 <gagehugo> lbragstad kmalloc was this resolved: https://bugs.launchpad.net/keystone/+bug/1778603 19:31:32 <openstack> Launchpad bug 1778603 in OpenStack Identity (keystone) "Documentation builds failing with Sphinx 1.7.5" [High,In progress] - Assigned to Morgan Fainberg (mdrnstm) 19:32:03 <lbragstad> i don't think so 19:32:45 <lbragstad> to work around is to not use v 1.7.5 19:32:56 <lbragstad> otherwise - it needs to be fixed upstream 19:33:07 <lbragstad> https://github.com/oauthlib/oauthlib/issues/558 19:36:07 <gagehugo> ah 19:37:19 <gagehugo> wasn't sure if the work-around was enough on our end 19:42:55 <lbragstad> we'll need to consume a version of oauthlib that fixes the issues i tink 19:42:57 <lbragstad> thin* 19:43:05 * lbragstad gives up 19:43:25 <lbragstad> i didn't want to spell that work correctly anyway 19:43:50 * lbragstad sigh 19:43:55 <gagehugo> lol 19:47:20 * cmurphy pats lbragstad on the head 20:43:01 <openstackgerrit> Lance Bragstad proposed openstack/keystone master: WIP: re-use bootstrap in tests https://review.openstack.org/564905 20:43:01 <openstackgerrit> Lance Bragstad proposed openstack/keystone master: WIP: Implement scope_type checking for credentials https://review.openstack.org/594547 20:43:17 <lbragstad> super super rough ^ 20:43:34 <lbragstad> but i'd appreciate any early feedback 20:43:42 <lbragstad> (the tests aren't very DRY) 20:44:09 <lbragstad> and i need to open a bug for that API 20:44:33 <lbragstad> but we should be able to do more things like that throughout stein hopefully, as apis convert to flask 20:45:30 <lbragstad> #endmeeting