| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2018-02-15 | |||
| 20:13:55 | rm_work | on my liberty cloud :/ | |
| 20:14:04 | rm_work | so it won't test the same issue, lol | |
| 20:14:15 | mordred | it will not - but the port list test should be good | |
| 20:14:36 | mordred | rm_work: I've been testing octavia listing load balancer against the released version running in vexxhost public cloud | |
| 20:14:48 | rm_work | on your branch, port list fails | |
| 20:15:00 | rm_work | octavia lb list works tho | |
| 20:15:03 | mordred | rm_work: awesome! at least it's reproducible :) | |
| 20:15:19 | rm_work | but again, i'm on like | |
| 20:15:25 | rm_work | the absolute newest possible | |
| 20:15:28 | mordred | rm_work: so on liberty does neutron just not have a version discovery doc? | |
| 20:15:35 | rm_work | i think that's correct | |
| 20:15:45 | rm_work | which somehow was fine still in 0.9.19 | |
| 20:15:51 | rm_work | but everything after that it explodes | |
| 20:16:03 | mordred | yah. major changes between 0.9.19 and 0.11 | |
| 20:16:13 | rm_work | :( | |
| 20:16:27 | mordred | lemme make a local test case and figure out why that's exploding | |
| 20:17:33 | mordred | rm_work: if you add 'network' to the tuple on line 327 in openstack/config/cloud_region.py - does that fix port list? | |
| 20:20:09 | rm_work | it does not | |
| 20:20:15 | mordred | rm_work: oh - I have reproduced the issue locally | |
| 20:20:21 | rm_work | ;) | |
| 20:24:27 | mordred | rm_work: one more thing - can you try adding "network_api_version": "2", to openstack/config/defaults.json ? | |
| 20:24:55 | mordred | rm_work: I believe that will still not work foryou on liberty - but I'd like to double-check | |
| 20:24:57 | rm_work | in addition to the other change? | |
| 20:25:05 | mordred | rm_work: other change not needed | |
| 20:26:06 | rm_work | k, yeah, still no worky | |
| 20:26:35 | mordred | kk. thanks. I grok the problem and will now commence to cry for a litle while then make a fix for you | |
| 20:32:48 | rm_work | thanks :) | |
| 20:59:27 | openstackgerrit | Monty Taylor proposed openstack/python-openstacksdk master: Handle liberty-era neutron https://review.openstack.org/545117 | |
| 21:05:36 | mordred | rm_work: ^^ if you have a sec, can you try that ^^ ? | |
| 21:07:45 | openstackgerrit | Brian Haley proposed openstack/python-openstackclient master: Add --dns-domain to 'network create' and 'network set' https://review.openstack.org/545124 | |
| 21:14:28 | openstackgerrit | Merged openstack/keystoneauth master: Split request logging into four different loggers https://review.openstack.org/505764 | |
| 21:14:30 | openstackgerrit | Merged openstack/keystoneauth master: Add some comments explaining split_loggers flag logic https://review.openstack.org/541066 | |
| 21:16:03 | rm_work | mordred: seems to work! :) | |
| 21:16:12 | rm_work | \o/ | |
| 21:16:16 | mordred | rm_work: woot! | |
| 21:23:05 | openstackgerrit | Merged openstack/keystoneauth master: Remove PYTHONHASHSEED setting https://review.openstack.org/533798 | |
| 21:31:20 | dasp | around in openstacksdk. I can propose a solution as you suggested but I wasn't sure how deep we want to go there. | |
| 21:31:20 | dasp | mordred: I wanted to follow up on https://review.openstack.org/#/c/543035/. I get your point but I'm not sure that it would be sufficient - some proxies expose APIs at a given path (e.g. vip:443/glance/v2/images) and then when the API service (glance in this case) is cooperating (some allow you to define base path), the links to pages would not start with /vX. Ideally, I think keystoneauth1 is better place to handle that instead of working | |
| 21:48:43 | mordred | dasp: so... hrm. this is a tricky interaction between how relative paths work for http and how 'mounted' adapters deal with them clashing :( | |
| 21:50:37 | mordred | dasp: there isn't really anything we can do at the keystoneauth layer, as the Adapter is mounted at the versioned endpoint. if the service is returning an absolute path but not a full url in the next links, then it's running in to requests Adapter treating it as rooted on the adapter mount point rather than rooted on the base of the http target | |
| 21:51:20 | mordred | dasp: so we may need to do our own joining at the point where your patch is ... | |
| 21:52:49 | mordred | dasp: basically - if the uri returned from the pagination startswith '/' - then we need to do a urljoin on it with the service's url ... in fact, we might be able to do that unconditionally | |
| 21:54:50 | mordred | dasp: SO - actually, I think your patch is almost right there - it's just that it needs to use urljoin instead of +/split - and it needs to use session.get_endpoint() instead of cls.base_path | |
| 22:03:28 | dasp | mordred: that I can do soon. Still it's not perfect, because it's common to expose glance at https://vip:443/images and the to list images, it's https://vip:443/images/v2/images and when we split it, we will erronously catch the first "/images". But I don't think there's any way around that here :/ | |
| 22:04:35 | mordred | dasp: I don't think we need to split it actually ... | |
| 22:05:18 | mordred | dasp: I *think* we can just take the url we get in uri and do urlparse.urljoin(session.get_endpoint(), uri) | |
| 22:09:45 | dasp | mordred: I'll test a few things and let you know, thanks for input | |
| 22:10:02 | mordred | dasp: http://paste.openstack.org/show/673168/ | |
| 22:10:19 | mordred | dasp: thanks for finding/fixing the issue! | |
| 22:11:34 | mordred | dasp: there's a place in keystoneauth (an also I think in shade somewhere) where we do extra logic to join version discovery links onto the url in the catalog because somethings (glance being a notable one) return broken links (like the internal ip address) | |
| 22:12:16 | mordred | dasp: it's possible we should extract that into a special join method in keystoneauth that we can use here - but also that keystoneauth can use when processing the version discovery links | |
| 22:29:28 | openstackgerrit | Merged openstack/keystoneauth master: Add pep8 import order validation https://review.openstack.org/545094 | |
| 22:45:26 | openstackgerrit | Monty Taylor proposed openstack/python-openstacksdk master: Update all test base classes to use base.TestCase https://review.openstack.org/540373 | |
| 22:45:26 | openstackgerrit | Monty Taylor proposed openstack/python-openstacksdk master: Update base test case to use base from oslotest https://review.openstack.org/540372 | |
| 22:45:27 | openstackgerrit | Monty Taylor proposed openstack/python-openstacksdk master: Handle liberty-era neutron https://review.openstack.org/545117 | |
| 22:45:27 | openstackgerrit | Monty Taylor proposed openstack/python-openstacksdk master: Honor service type aliases from the catalog https://review.openstack.org/544356 | |
| #openstack-sdks - 2018-02-16 | |||
| 00:22:27 | openstackgerrit | Monty Taylor proposed openstack/python-openstacksdk master: Work around discovery being a bit messy for cinder https://review.openstack.org/545174 | |
| 00:26:12 | openstackgerrit | Daniel Speichert proposed openstack/python-openstacksdk master: Strip API version from subsequent paginated links https://review.openstack.org/543035 | |
| 00:39:23 | openstackgerrit | Daniel Speichert proposed openstack/python-openstacksdk master: It seems to be working as expected for glance pagination. https://review.openstack.org/545175 | |
| 02:36:55 | openstackgerrit | OpenStack Proposal Bot proposed openstack/openstackclient master: Updated from global requirements https://review.openstack.org/521902 | |
| 02:49:38 | openstackgerrit | OpenStack Proposal Bot proposed openstack/python-openstackclient master: Updated from global requirements https://review.openstack.org/544222 | |
| 14:38:47 | openstackgerrit | Monty Taylor proposed openstack/python-openstacksdk master: Honor service type aliases from the catalog https://review.openstack.org/544356 | |
| 17:16:22 | dasp | mordred: I updated https://review.openstack.org/#/c/543035/ and the fix works for Glance. Should I worry about failing tests? Do you think there's a way I can test pagination here? I'm not entirely sure what integration testing is in place but if there was a test with paginated image listing, we probably could have caught the issue. | |
| 18:45:07 | openstackgerrit | Monty Taylor proposed openstack/python-openstacksdk master: Update base test case to use base from oslotest https://review.openstack.org/540372 | |
| 18:45:08 | openstackgerrit | Monty Taylor proposed openstack/python-openstacksdk master: Honor service type aliases from the catalog https://review.openstack.org/544356 | |
| 18:45:08 | openstackgerrit | Monty Taylor proposed openstack/python-openstacksdk master: Update all test base classes to use base.TestCase https://review.openstack.org/540373 | |
| 18:46:26 | mordred | dasp: yeah - we should fix the tests, that's a real error. we've got functional tests in place ... but we'd need to get enough images loaded to cause pagination to be returned which I think would be hard on an 8G devstack | |
| 18:47:18 | mordred | dasp: I think for now we should just add a couple of requests-mock tests that return pagination links in all of the combinations | |
| 18:47:34 | mordred | dasp: I'll see if I can't knock up an example of that ... | |
| 18:49:44 | dasp | mordred: makes sense. We can just create 200 fake image with random dd'ed payload of 1 MB each. | |
| 18:51:33 | dasp | mordred: not sure if you've seen my two other changes proposed but they're about properties. I'm not sure if the current approach of adding every possible property added in source as a "known" property is to be continued. There are countless options and metadata definition is really for that. So in one change I proposed adding a few and in another, I fixed loading properties property with "everything else", since Glance image object is | |
| 18:51:34 | dasp | flattened. Is there any established direction for what's the better approach? | |
| 18:57:30 | openstackgerrit | Vladyslav Drok proposed openstack/osc-lib master: Allow to use the none auth plugin https://review.openstack.org/512699 | |
| 19:03:00 | mordred | dasp: I like the 200 fake images with 1M dd payload idea | |
| 19:07:11 | mordred | dasp: will look at the properties patches next - in the shade layer we put everything we don't know about into a 'properties' dict (the flat image object in glance v2 is ... so much fun) | |
| 19:07:57 | mordred | dasp: incidentally, http://paste.openstack.org/show/674819/ fixes the unit tests - but I think I may have been wrong about urljoin being good enough - and we may have to get more clever - so I think definitely getting some testing in place to make sure the right thing happens is going to be essential for this one | |
| 19:11:52 | mordred | dasp: like- something more akin to your first version - but that just does: if uri.startswith('/v') and uri[2].isdigit(): uri = uri.split('/', 2)[2] | |
| 19:15:28 | mordred | dasp: yeah, I like the idea in 544544 - haven't dug in to the impl details yet. also like 544049 | |
| 19:16:10 | mordred | dasp: in general I think we shold support reasonable/defined image properties like you did in 544049 - and have everything else go to properties | |
| 19:17:54 | dasp | mordred: awesome. Those were drafts to get the job done, so I expect some improvements may need to be applied but it worked for what I needed while writing a tool that uses it. | |
| 19:18:05 | mordred | dasp: I'm tempted to say we should just adopt the shade approach globally and put unknown fields into a generic properties bucket for everything - bu tI haven't actually thought through the ramifications for that, so I think it's safer to do it on image first | |
| 19:19:50 | dasp | mordred: there's another bug that I haven't brought up yet - openstacksdk is sending properties as JSON booleans whenever they are booleans. Glance API is kindly throwing 400. It wants 'true' to be a string instead (I found that out tracing what openstackclient is doing). So whenever I declared the property to be a bool, it's not working as expected due to Glance. | |
| 19:20:32 | dasp | mordred: Then, however, if I assign a string to a property declared as dict, it is actually sent to API as string, so I started to doubt if that type setting does anything at all. | |
| 19:21:07 | dasp | mordred: What I noticed openstackclient is doing is that it always asks for metadata catalog before updating images, then it probably validates client-side and figured out the type for every property. | |
| 19:22:55 | mordred | dasp: I believe the type setting is only used when we're injesting values from remotely - so yeah, it's not going to coerce it if you just set it | |
| 19:23:22 | mordred | dasp: hrm. we should maybe do the same thing as osc for glance and grab the metadata catalog for valdation/coercion | |
| 19:24:37 | mordred | dasp: although now I'm also wanting to have the properties coerce things when you set them - and figure out a 'good' way to be able to specify split typing - like "treat this as a bool for sdk interactoins, but send it as a string please" | |
| 19:26:16 | dasp | mordred: it sounds like it may be needed, although I wish that Glance API could accept booleans/floats even if the property needs to be stored as string later on internally. | |
| 19:41:43 | mordred | dasp: ++ | |
| 19:42:12 | mordred | dasp: of course, the joy here in sdk land is that even if we got glance fixed, we'd still need to support older unfixed clouds. yay us! | |
| 21:21:21 | openstackgerrit | Brianna Poulos proposed openstack/python-openstackclient master: Update help text for encryption provider https://review.openstack.org/545459 | |
| 23:23:33 | openstackgerrit | Monty Taylor proposed openstack/python-openstacksdk master: Update all test base classes to use base.TestCase https://review.openstack.org/540373 | |
| 23:23:33 | openstackgerrit | Monty Taylor proposed openstack/python-openstacksdk master: Update base test case to use base from oslotest https://review.openstack.org/540372 | |
| 23:23:34 | openstackgerrit | Monty Taylor proposed openstack/python-openstacksdk master: Honor service type aliases from the catalog https://review.openstack.org/544356 | |
| #openstack-sdks - 2018-02-17 | |||
| 00:17:50 | mordred | Qiming: I said this in the review - but perhaps we should set up a zaqar devstack config so we can get functional tests running (and catch things like the message/messaging issue) | |
| 05:20:41 | openstackgerrit | Monty Taylor proposed openstack/python-openstacksdk master: Update all test base classes to use base.TestCase https://review.openstack.org/540373 | |
| 05:20:41 | openstackgerrit | Monty Taylor proposed openstack/python-openstacksdk master: Update base test case to use base from oslotest https://review.openstack.org/540372 | |
| 05:20:42 | openstackgerrit | Monty Taylor proposed openstack/python-openstacksdk master: Honor service type aliases from the catalog https://review.openstack.org/544356 | |
| 05:20:59 | openstackgerrit | Monty Taylor proposed openstack/python-openstacksdk master: Run examples tests with functional tests https://review.openstack.org/540374 | |
| 05:21:04 | openstackgerrit | Monty Taylor proposed openstack/python-openstacksdk master: Re-enable orchestration functional tests https://review.openstack.org/540412 | |