Earlier  
Later
New Search
Posted Nick Remark
#openstack-sdks - 2018-06-13
14:44:26 mordred well - sure
14:44:39 cdent for modern things that don't do such blasphemy, if no link rel self then endpoint is what you already requested
14:45:05 cdent that was supposed to come out as a question, not a dammit!
14:45:41 cdent mordred: i can stick it in too, to that same patch, but it seems...weird
14:45:55 mordred that would be a potential behavior change in keystoneauth ... https://github.com/openstack/keystoneauth/blob/master/keystoneauth1/discover.py#L543-L547
14:46:31 mordred kmalloc: ^^ if we stopped skipping entries with no self link and used that to infer that the existing endpoint was the self-link - would you consider that a breaking change to keystoneauth?
14:46:49 cdent i'm happy to fix placement to make life easier
14:47:05 cdent but I think we want to make room for a future where endpoints are endpoints and that's it
14:47:18 kmalloc mordred: hm
14:47:21 kmalloc reading up
14:48:26 kmalloc mordred: it would be a breaking change, behavior changes are breaking =/
14:48:50 mordred nod. we can queue that up for keystoneauth2
14:49:07 cdent still confused on how it is currently working for nova?
14:49:20 mordred cdent: that is honestly a fascinating question
14:49:22 kmalloc yeah. i am inclined to say we should ksa2 at this point and drop some behavior
14:49:24 cdent s/still/I am still/
14:49:43 kmalloc well... if it has links but no self
14:49:46 kmalloc it doesn't skip
14:49:53 cdent no links at all
14:49:57 kmalloc hm.
14:50:08 cdent so maybe the problem is not at the ksa level?
14:50:20 kmalloc maybe.
14:50:23 cdent in which case I'd argue against changing placement /, perhaps?
14:50:41 mordred cdent: yah - it might be that sdk is triggering version discovery where nova + plain ksa is not
14:50:51 kmalloc likely
14:51:07 kmalloc sdk does stuff in some more indirect ways.
14:51:15 mordred cdent: I would prefer to not special-case placement in sdk with a "please don't try version discovery on placement" - also, it'll break discovering available microversions
14:51:45 kmalloc mordred: ftr, i would love to stop skipping no-link items
14:51:47 cdent i'm not suggesting to avoid version discovery
14:51:56 kmalloc i just see some "this might actually break someone"
14:52:00 kmalloc potential
14:52:03 cdent well if you both feel that way I'll add a link
14:52:14 cdent I don't feel strongly about it
14:52:14 mordred cdent: yah - but nova's use of ksa is really only likely able to work by using ksa in such a way that version discovery is being avoided
14:52:24 kmalloc cdent: i don't want you to add a link, i don't think we can change ksa's behavior
14:52:30 mordred yah - I think the link is silly
14:52:33 kmalloc cdent: so, adding a link might be the correct way
14:52:45 kmalloc just not the desirable one
14:53:08 cdent what I do feel strongly about is having version numbers in urls and double endpoints
14:53:13 kmalloc mordred: i dislike the volume of inherited ick we carried over from ksc.session
14:53:18 mordred kmalloc: yah
14:53:23 mordred cdent: totally.
14:53:23 cdent but we are in the reality we are in so...
14:53:28 kmalloc cdent: please please please don't do version numbers in urls if you can avoid.
14:53:38 kmalloc cdent: so.. i am with you there
14:53:58 kmalloc mordred: how bad would it be to cut a ksa2 and drop compat behaviors [for sdk]
14:54:00 cdent kmalloc: yeah, don't plan to: placement was designed from the outset to never do that
14:54:10 kmalloc and start iterating on that instead of ksa1 continues?
14:54:21 kmalloc cdent: ++ and i appreciate that so very much!
14:54:23 mordred kmalloc: I don't think it would be _bad_ - but I think it would take us a few months
14:54:49 kmalloc mordred: well we'd need to ksa2 work anyway. i'm inclined to talk to lbragstad about it
14:54:56 mordred kk
14:55:04 kmalloc ksa1 is pretty stable, and we could make ksa2 just without all the icky compat stuff.
14:55:23 cdent can you guys speculate what ksa or the sdk would do if I set the href for the self link to ''
14:55:41 mordred I'll show you the code
14:56:04 cdent I want to do that because no where in placement does placemet know its own scheme, hostname, port, base url. And it is very important that it stay that way
14:56:36 cdent sorry about my n key
14:56:40 mordred cdent: https://github.com/openstack/keystoneauth/blob/master/keystoneauth1/discover.py#L406-L426
14:57:42 mordred (although there is a bug in that that needs fixing)
14:57:50 cdent I read that as "should be okay": it will result in the discovery url, which is what we want
14:58:22 mordred cdent: yes. I believe that is true - also, we can make mnaser deploy it and we can find out for sure ;)
14:58:43 kmalloc cdent: it should work afaict.
14:59:08 kmalloc but, honestly,sometimes discovery stuff is wonkier than expected -- we have a lot of carry-over magic in lots of places.
14:59:23 cdent yeah, we shall see
15:00:39 mordred cdent, kmalloc: yup. this is the result of having to support the epic amount of incompatible pre-existing mush
15:01:01 mnaser i don't mind making changes if they're not too wild
15:02:50 mordred mnaser: btw - placement_enpdoint_override just worked for me
15:02:57 mordred placement_endpoint_override: http://placement-ca-ymq-1.vexxhost.net/
15:03:07 mnaser maybe because i'm using os_client_config i guess
15:03:11 mnaser wait wait wait
15:03:18 mnaser mordred: remember the bug i mentioned about endpoint override not working?!
15:03:26 mnaser you fixed it but we might not had a release since
15:03:29 mordred yah
15:03:31 mordred ah
15:03:37 mordred yah - we need a release
15:03:47 mnaser https://review.openstack.org/#/c/565489/
15:08:10 frickler mordred: nothing obvious on https://review.openstack.org/#/c/557935/ for me, either. though I seem to remember having seen this fail earlier with what I suspected to be race conditions, but on the detach side
15:09:03 frickler mordred: might either be an unstable test and the two in a row just unlucky, or some subtle change in timing somewhere
15:09:36 mordred frickler: nod. I'll recheck again just to be sure
15:10:26 frickler mordred: yeah, if that still has the same failure, I'll start comparing versions with the passing runs earlier
15:14:40 cdent woot on 565489
15:19:45 cdent mordred, mnaser, kmalloc https://review.openstack.org/575117 updated to add links
15:22:30 mnaser if that seems okay to mordred and other reviews i can push that up to test
15:25:09 mordred mnaser: I think it's worth trying - also, we totally need to make an sdk release
15:27:22 mnaser mordred: ill wait for some nova reviews to roll in before i make that change
15:32:57 kmalloc cdent: ++
16:16:49 openstackgerrit Monty Taylor proposed openstack/openstacksdk master: Add test to verify behavior on '' in self link https://review.openstack.org/575158
16:17:21 mordred cdent, kmalloc, mnaser: ^^ that satisfies me that the link of '' from placement will do the right thing from sdk
16:17:47 kmalloc mordred: are we cross-gating sdk and ksa?
16:17:54 kmalloc if not, lets get that encoded in a test in KSA too
16:18:01 kmalloc i don't want to accidently break someone down the line.
16:18:15 mordred we are
16:18:17 kmalloc because the behavior changes because of some other "fix" that we miss.
16:18:20 kmalloc ok cool
16:18:23 kmalloc wfm then
16:18:24 mordred and yes - I think a test for that case in ksa is a good idea
16:18:31 mordred but also we cross-gate :)
16:18:51 kmalloc cross-gate makes me happy. i wont ever say no to more quality behavior tests ;)
16:19:33 openstackgerrit Monty Taylor proposed openstack/openstacksdk master: Add tests to verify behavior on '' in self link https://review.openstack.org/575158
16:19:49 mordred cdent: ^^ added a second test just to verify microversion info is found correctly

Earlier   Later