| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-sdks - 2018-06-13 | |||
| 14:43:56 | mordred | $ curl http://compute-ca-ymq-1.vexxhost.net/ | |
| 14:43:57 | mordred | {"versions": [{"status": "SUPPORTED", "updated": "2011-01-21T11:33:21Z", "links": [{"href": "http://compute-ca-ymq-1.vexxhost.net/v2/", "rel": "self"}], "min_version": "", "version": "", "id": "v2.0"}, {"status": "CURRENT", "updated": "2013-07-23T11:33:21Z", "links": [{"href": "http://compute-ca-ymq-1.vexxhost.net/v2.1/", "rel": "self"}], "min_version": "2.1", "version": "2.60", "id": "v2.1"}]} | |
| 14:44:17 | cdent | because they are hilariously antiquated things that do weird things like put versions in urls on the same service endpoint for | |
| 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 :) | |