| *** slaweq has joined #openstack-shade | 00:45 | |
| *** slaweq has quit IRC | 00:51 | |
| *** gkadam has joined #openstack-shade | 03:48 | |
| *** gouthamr has quit IRC | 05:41 | |
| *** slaweq has joined #openstack-shade | 05:51 | |
| *** slaweq has quit IRC | 05:56 | |
| *** slaweq has joined #openstack-shade | 06:52 | |
| *** slaweq has quit IRC | 06:57 | |
| *** slaweq has joined #openstack-shade | 07:03 | |
| *** ioggstream has joined #openstack-shade | 07:44 | |
| *** slaweq has quit IRC | 08:09 | |
| *** slaweq has joined #openstack-shade | 08:09 | |
| *** slaweq has quit IRC | 08:20 | |
| *** slaweq has joined #openstack-shade | 09:21 | |
| *** slaweq has quit IRC | 09:26 | |
| *** slaweq has joined #openstack-shade | 10:22 | |
| *** slaweq has quit IRC | 10:27 | |
| *** slaweq has joined #openstack-shade | 11:41 | |
| *** slaweq has quit IRC | 11:48 | |
| *** gkadam has quit IRC | 12:02 | |
| samueldmq | morning shade | 12:05 |
|---|---|---|
| mordred | morning samueldmq ! | 13:01 |
| samueldmq | mordred: o/ | 13:01 |
| samueldmq | mordred: I found a bug in shade | 13:07 |
| samueldmq | 404's are turned into OpenStackCloudURINotFound, which is not *always* true, e.g user not found in group returns 404 | 13:08 |
| samueldmq | e.g http://logs.openstack.org/57/499357/4/check/gate-shade-functional/4ac1cda/console.html#_2017-09-02_21_12_33_390273 | 13:08 |
| mordred | samueldmq: well, that's one of the downsides to REST - a 404 is, by definition, URI Not Found. :) | 13:12 |
| samueldmq | D: | 13:12 |
| mordred | samueldmq: however, before we were catching keystoneauth1.exceptions.http.NotFound in that method, and now we just need to catch OpenStackCloudURINotFound instead | 13:13 |
| mordred | (and the other two excepts there can go away, they're not needed anymore) | 13:14 |
| samueldmq | mordred: ok so I can handle taht and add a comment "we're sure this url exists ..... so a 404 means user not in group in this case ...." | 13:14 |
| mordred | ++ | 13:14 |
| samueldmq | mordred: I also need to check on specific entities that do not exist in v2.0. e.g groups | 13:15 |
| samueldmq | calling is_user_in_group when only v2.0 is available should throw an exception, not just say "false" | 13:15 |
| mordred | samueldmq: agree | 13:16 |
| mordred | oh - also - we have the ability to do something now | 13:16 |
| mordred | samueldmq: (I'd almost forgotten why getting shade to consume ksa version discovery was so important ...) | 13:16 |
| openstackgerrit | Monty Taylor proposed openstack-infra/shade master: WIP Force keystone v2 admin call to admin interface https://review.openstack.org/500540 | 13:17 |
| mordred | samueldmq: this ^^ | 13:17 |
| mordred | samueldmq: the admin calls in keystone v2 are all supposed to be made against the admin endpoint | 13:17 |
| mordred | we didn't have a way to force that before, because of how we were making the client objects | 13:18 |
| mordred | samueldmq: but with ksa and letting ksa do the versiondiscovery for us - we can now just add an endpoint_filter={'interface': 'admin'} to calls that need to be v2 admin | 13:18 |
| samueldmq | mordred: well, we've been doing some of that already | 13:18 |
| samueldmq | e.g https://review.openstack.org/#/c/482197/3/shade/operatorcloud.py | 13:18 |
| samueldmq | :) | 13:19 |
| mordred | samueldmq: oh good ... and it worked? :) | 13:19 |
| samueldmq | mordred: I trusted you :-) how can I check it? | 13:19 |
| mordred | oh! duh. this has been possible with keystone for a while ... because keystoneauth has known how to do version discovery for keystone the whole time | 13:20 |
| * mordred is still waking up | 13:20 | |
| samueldmq | :) | 13:32 |
| openstackgerrit | Samuel de Medeiros Queiroz proposed openstack-infra/shade master: De-client-ify Check User in Group https://review.openstack.org/499357 | 13:39 |
| openstackgerrit | Samuel de Medeiros Queiroz proposed openstack-infra/shade master: De-client-ify Check User in Group https://review.openstack.org/499357 | 13:42 |
| openstackgerrit | Samuel de Medeiros Queiroz proposed openstack-infra/shade master: De-client-ify Remove User from Group https://review.openstack.org/499360 | 13:43 |
| *** slaweq has joined #openstack-shade | 13:50 | |
| mordred | samueldmq: WE'RE SO CLOSE | 13:54 |
| samueldmq | mordred: :D and what's the missing bit? | 13:54 |
| *** slaweq has quit IRC | 13:55 | |
| mordred | samueldmq: if you feel like it, in a follow up patch I think in https://review.openstack.org/#/c/499357/6/shade/openstackcloud.py you can also delete the except Exception: clause - the only exception that should be thrown from that is from the adapter already | 13:57 |
| mordred | but the patch looks great | 13:57 |
| mordred | samueldmq: roles and endpoints are the only ones left | 13:58 |
| samueldmq | mordred: ++ I am looking at them, hopefully I can get the rest of the patches this week | 14:00 |
| openstackgerrit | Monty Taylor proposed openstack-infra/shade master: Remove use of legacy keystone client in functional tests https://review.openstack.org/500558 | 14:02 |
| openstackgerrit | Samuel de Medeiros Queiroz proposed openstack-infra/shade master: Remove improper exc handling in is_user_in_group https://review.openstack.org/500560 | 14:03 |
| samueldmq | mordred: ^ | 14:03 |
| samueldmq | sweet! | 14:03 |
| mordred | TheJulia: you saw the use-new-ksa patch landed right? so you can now call "self._baremetal_client.get_endpoint_data" and get an object that will tell you min and max microversion available - and you can pass microversion= in all of your rest calls - like self._baremetal_client.get('/foo', microversion='1.10') | 14:39 |
| TheJulia | No I didn't see that, but awesome. Out of curiosity, what does it default to if no version is defined? | 14:42 |
| TheJulia | the client constructor, that is | 14:43 |
| openstackgerrit | Merged openstack-infra/shade master: De-client-ify User Update https://review.openstack.org/499284 | 14:45 |
| mordred | TheJulia: it defaults to not sending any microversions | 14:46 |
| mordred | TheJulia: actually - let me restate that ... | 14:46 |
| mordred | TheJulia: the client constructor by default has no relationship with microversions at all | 14:46 |
| mordred | becaues microversions are a per-REST call construct | 14:47 |
| mordred | HOWEVER | 14:47 |
| mordred | you can pass a default_microversion parameter to the Adapter constructor which will cause it to behave as-if you had added microversion=blah to each REST call | 14:47 |
| mordred | I think this mightbe a thing we want to do for ironic since you have a default min microversion you need, right? | 14:48 |
| TheJulia | mordred: yeah, and I take it that explicitly stating it overrides the default? | 14:48 |
| *** slaweq has joined #openstack-shade | 14:51 | |
| * TheJulia ponders | 14:55 | |
| *** slaweq has quit IRC | 14:56 | |
| mordred | yes | 15:08 |
| mordred | TheJulia: explicitly stating always wins | 15:08 |
| mordred | TheJulia: the overall intent is that in the general case specifying default_microversion is not what you should do - as it's a per-call thing, generally speaking you want to say "I want version 1.15 of GET /machines" | 15:09 |
| mordred | BUT ... that's theory - in practice there are things like "zomg never use less than ironic microversion 1.3" | 15:09 |
| mordred | or even "shade does not support ironic that doesn't have at least microversion 1.3" | 15:10 |
| mordred | and if that was the case, we could do a check for min_microversion from self._baremetal_client.get_endpoint_data when we create self._baremetal_client and throw an exception if the cloud doens't have it | 15:10 |
| mordred | because better do that upfront and say "sorry, your cloud is too old we literally cannot deal with it" | 15:10 |
| mordred | than to get halfway into an operation then hit the point where 1.3 is super necessary and we can't deal with it not being there and THEN tell people they're hosed | 15:11 |
| TheJulia | yeah, microversions make it _fun_. Not sure the best way to do it efficently, although there is still the cost of negotiating a desired microversion. Perhaps thats a something to add and leverage moving forward.... :\ | 15:11 |
| TheJulia | yeah | 15:11 |
| mordred | TheJulia: this is an interesting concept in the context of shade, since for us we don't want to expose them to users, we want to use them to find if we can do more advanced things | 15:11 |
| mordred | TheJulia: well - 2 things on negotiation - one is that the version info is cached at the top of the session,so we only fetch it once during version discovery | 15:12 |
| TheJulia | hmmmmm | 15:13 |
| mordred | TheJulia: that said, I have a spec I need to update that involves providing an extremely cachable cloud-level document with all of the microversion of all of the services - although that doesn't hepl the bifrost case | 15:13 |
| TheJulia | yeah, a couple variables to consider | 15:14 |
| mordred | TheJulia: we could also add support for locally caching version documents based on time and/or etag | 15:14 |
| mordred | it's a bunch of fun | 15:14 |
| mordred | for now I'm mostly trying to make sure that we have the consumption api in keystoneauth so that getting available versions and then requesting one per request works | 15:14 |
| mordred | even if it's not 100% as efficient as it could be | 15:14 |
| mordred | caues then we can improve caching/efficiency under the covers in ksa | 15:15 |
| mordred | and get the logic right once | 15:15 |
| openstackgerrit | Paul Belanger proposed openstack-infra/shade master: Add openstack-doc-build to shade https://review.openstack.org/500201 | 15:27 |
| *** gkadam has joined #openstack-shade | 15:35 | |
| *** gouthamr has joined #openstack-shade | 15:37 | |
| *** slaweq has joined #openstack-shade | 15:52 | |
| *** slaweq has quit IRC | 15:56 | |
| *** gkadam has quit IRC | 16:50 | |
| *** gkadam has joined #openstack-shade | 16:51 | |
| *** slaweq has joined #openstack-shade | 16:53 | |
| TheJulia | mordred: I feel like the definition of "fun" needs to be checked/verified. | 16:54 |
| *** slaweq has quit IRC | 16:58 | |
| *** gkadam has quit IRC | 17:07 | |
| *** slaweq has joined #openstack-shade | 17:54 | |
| mordred | TheJulia: :) | 17:56 |
| *** slaweq has quit IRC | 17:58 | |
| *** gouthamr has quit IRC | 18:19 | |
| *** ioggstream has quit IRC | 18:19 | |
| *** gouthamr has joined #openstack-shade | 18:20 | |
| * TheJulia finally starts pondering microversions again | 18:29 | |
| *** kmalloc_ has joined #openstack-shade | 18:30 | |
| *** slaweq has joined #openstack-shade | 18:31 | |
| *** slaweq has quit IRC | 18:32 | |
| *** kmalloc has quit IRC | 18:37 | |
| *** RuiChen has quit IRC | 18:37 | |
| *** kmalloc_ is now known as kmalloc | 18:37 | |
| *** pabelanger has quit IRC | 19:17 | |
| *** pabelanger has joined #openstack-shade | 19:17 | |
| openstackgerrit | Monty Taylor proposed openstack-infra/shade master: De-client-ify Add User to Group https://review.openstack.org/499345 | 19:20 |
| openstackgerrit | Monty Taylor proposed openstack-infra/shade master: De-client-ify Check User in Group https://review.openstack.org/499357 | 19:21 |
| openstackgerrit | Monty Taylor proposed openstack-infra/shade master: De-client-ify Remove User from Group https://review.openstack.org/499360 | 19:21 |
| openstackgerrit | Monty Taylor proposed openstack-infra/shade master: Remove use of legacy keystone client in functional tests https://review.openstack.org/500558 | 19:21 |
| openstackgerrit | Monty Taylor proposed openstack-infra/shade master: Remove improper exc handling in is_user_in_group https://review.openstack.org/500560 | 19:21 |
| *** slaweq has joined #openstack-shade | 19:33 | |
| *** slaweq has quit IRC | 19:38 | |
| *** ioggstream has joined #openstack-shade | 19:45 | |
| *** RuiChen has joined #openstack-shade | 20:25 | |
| *** slaweq has joined #openstack-shade | 20:34 | |
| *** slaweq has quit IRC | 20:39 | |
| *** slaweq has joined #openstack-shade | 20:42 | |
| *** slaweq has quit IRC | 21:02 | |
| openstackgerrit | Merged openstack-infra/shade master: De-client-ify Add User to Group https://review.openstack.org/499345 | 21:05 |
| *** ioggstream has quit IRC | 21:14 | |
| TheJulia | mordred: so it looks like version discovery would just have to become the default (looks like it is presently false) to get the top level stored data in the session, and then each high level call in shade could just call an internal helper could be called with some minimum data that in a sense could actually provide very verbose error messaging about minimum versions if the endpoint is insufficient. Aside | 21:18 |
| TheJulia | from that, the client can just be a versioned client attempting to negotiate version.latest which avoids minimum versions being pulled and explicitly errored upon one day. | 21:18 |
| TheJulia | shade just has to have the logic, to navigate breaking changes, or preference behavior on the cached data | 21:19 |
| mordred | TheJulia: totally | 21:19 |
| TheJulia | actually, I think it could be invoked in get_endpoint, if I'm remembering correctly.... | 21:25 |
| * TheJulia is not seeing a downside or a horrible caveat | 21:25 | |
| mordred | TheJulia: so -because I know you want more work on your plate ... | 21:44 |
| TheJulia | .... | 21:44 |
| TheJulia | can this be over some sort of tasty beverage in denver? | 21:44 |
| mordred | :) | 21:44 |
| TheJulia | speaking of denver... | 21:45 |
| TheJulia | the wind is putting new mexico to shame | 21:45 |
| mordred | TheJulia: instead of mocking the baremetal_client, what would be great is converting the mocks of ironic_client in shade/tests/unit/test_shade_operator.py to use requests_mock - that way it's mocking at the low-level requests Session level | 21:45 |
| TheJulia | yeah.... that has been in the back of my brain. I need to wrap my head around it further | 21:46 |
| mordred | https://review.openstack.org/#/c/474841 is an example of having done it for some nova calls | 21:47 |
| mordred | (I'm an example person myself) | 21:47 |
| mordred | main key is that you call self.register_uris for the test with a list of the REST calls you expect the test to make - then remove all mocking of ironic_client methods, and requests_mock does the rest | 21:48 |
| TheJulia | \o/ an example | 21:48 |
| mordred | (self.assert_calls() at the end ensures that all the calls got made and in the order you expected) | 21:48 |
| TheJulia | I guess that works, and as the logic changes, evolve the expected calls | 21:49 |
| mordred | https://review.openstack.org/#/c/474841/3/shade/tests/unit/test_server_delete_metadata.py is a good file to look at | 21:49 |
| mordred | yah. it's helpful as part of this process because you can update the test underneath ironicclient - so you can be sure the new test is correct - then when you change the code to move from ironic_client to baremetal_client - the test should be able to not change in the same patch | 21:50 |
| mordred | oh - https://review.openstack.org/#/c/474841/3/shade/tests/unit/test_server_set_metadata.py has an exampleof a thing that's good to know too ... | 21:50 |
| mordred | in test_server_set_metadata_with_exception you'll see the call then a "validate=dict(" thing - that is used to validate payloads sent to the cloud | 21:51 |
| mordred | so dict(method='GET', uri=self.get_mock_url('compute', 'public', append=['servers', 'detail']),json={'servers': [self.fake_server]}) says "if I do GET /servers/detail on the public endpoint of the compute service - return {'servers': [self.fake_server]} | 21:52 |
| mordred | but the validate= call says, return {} to the library when I make that POST call, but make sure the library sent {'metadata': {'meta': 'data'}} as the payload | 21:53 |
| mordred | (and you can do headers and url params and whatnot too, if they're important) | 21:53 |
| TheJulia | hmm, okay, I guess I was trying to avoid hooking the existing client up to it, but it does make sense. | 21:54 |
| openstackgerrit | Merged openstack-infra/shade master: De-client-ify Check User in Group https://review.openstack.org/499357 | 22:59 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!