Earlier  
Later
New Search
Posted Nick Remark
#openstack-sdks - 2017-07-06
16:02:06 elmiko edleafe, cdent, are we meeting today?
16:02:17 cdent oh yeah
16:02:21 cdent my calendar didn't warn me
16:02:22 elmiko XD
16:02:28 elmiko i started the meeting already
16:26:52 cdent elmiko, edleafe : https://etherpad.openstack.org/p/api-wg-newsletter
16:28:13 elmiko cdent: lgtm
16:28:14 elmiko thanks
16:28:46 edleafe cdent: Not as witty as your usual output, but given your layoff, acceptable
16:29:02 cdent cheers, I'll try to do better next time
16:33:42 efried mordred Are three-part version numbers (e.g. v1.2.3) okay or not?
16:33:52 efried or four, or five...?
17:25:40 openstackgerrit Akihiro Motoki proposed openstack/python-openstackclient master: Network tag support https://review.openstack.org/461195
17:42:51 amotoki is there a plan to release a new version of cliff?
17:44:27 amotoki osc and neutronclient (possibly other osc plugins) can use the sphinxext to generate CLI reference automatically. it reduces developer boring effort a lot.
17:53:45 openstackgerrit Akihiro Motoki proposed openstack/python-openstackclient master: Use openstackdocstheme in release note https://review.openstack.org/481173
18:13:55 openstackgerrit Akihiro Motoki proposed openstack/python-openstackclient master: Use openstackdocstheme in release note https://review.openstack.org/481173
18:57:55 dtroyer amotoki: yes, soon. I've got a question on https://review.openstack.org/479157 yet
18:58:03 dtroyer dhellmann: any opinions on ^^^^ ??
18:58:06 mordred efried: they are not
18:58:40 dhellmann dtroyer : looking
18:59:09 efried mordred Okay. Also looking at other (lack of) validation in discover.py
18:59:47 mordred efried: ah - so - discover.py should likely be more permissive if it sensibly can
19:00:24 mordred efried: I don't think a service returning a three-part version should break discover.py - but I also think we should be clear in docs that it's not 'supported'
19:00:49 efried mordred I'm working on a change to address some of the (post-merge) comments I put on https://review.openstack.org/#/c/469087/14
19:01:16 mordred efried: woot!
19:01:36 efried mordred How would you feel about a log warning if len(version_tuple) != 2 ?
19:02:06 dtroyer osc meeting in #openstack-meeting if anyone is around...
19:11:30 mordred efried: as long as it's squelchable - the big isssue is as a consumer of a cloud, if the cloud decides for whatever reason to return a version triple, I can't actually do anything about it, and now I've got warnings spamming me
19:11:59 efried mordred Mm, fair enough.
19:12:23 mordred efried: as a real-world example of this - there's a warning in python-requests about subjectaltname in ssl certs that gets tripped by the certs deployed on rackspace public cloud
19:12:45 mordred efried: which is why we have the requestsexceptions lib - which has an easy-button for making that go away
19:15:56 stevemar amotoki: +2! just need dtroyer to look at it now :)
19:16:37 dtroyer stevemar: join us in #openstack-meeting if you have a minute
19:17:28 openstackgerrit Doug Hellmann proposed openstack/cliff master: add tests for display command classes and hooks https://review.openstack.org/481192
19:22:18 efried mordred But version tuples should have at *least* two parts, yes?
19:22:46 efried Today if you normalize_version_number([1]) you get (1,)
19:34:06 mordred efried: you'd think - but no - versions in urls often only have one number (v2)
19:34:13 openstackgerrit OpenStack Proposal Bot proposed openstack/os-api-ref master: Updated from global requirements https://review.openstack.org/478040
19:34:30 mordred efried: and the normalize_version code is used as part of matching requested versions with found versions
19:34:30 efried mordred But in discover.py, you're specifically translating "v2" to (2, 0)
19:34:34 mordred yup
19:34:48 mordred we can't change that- it's pre-existing keystoneauth behavior
19:35:05 efried So you want "v2" => (2, 0) and "2" => (2, 0) but [2] => (2,) ?
19:35:27 mordred hrm. no - that seems like a bug
19:35:36 efried okay, cool, swhat I'm fixing.
19:35:40 mordred kk :)
19:36:33 efried mordred Honestly, I'm having trouble keeping straight which pieces of data are coming from where, and where they're going to.
19:37:50 mordred efried: yah- doing a refactor inside of discovery.py is overdue
19:38:37 efried mordred Here's another poser for ya.
19:41:12 efried mordred In Discover.version_data - if max_version is set, but "" (empty string) or 0 or None, do we want to behave same as if it's absent (i.e. default to using the `version` field) or use actual None, which (I think) equates to `latest` ?
19:43:01 efried mordred I would think we would want to do the former (especially if `version` is present). But today I believe we're doing the latter.
19:43:31 efried Because {'key': None}.get('key', 'default') returns None, not 'default'.
19:45:57 mordred efried: if max_version and version are both set, that's an error. if max_version is set but is set to "" or 0 - I think that's super confusing and we should throw an error
19:46:26 mordred efried: and we haven't released with max_version yet - so I think making that change should be fine
19:46:56 efried mordred Okay, none of that is throwing an error today. So can I add those checks (for mutual exclusivity and bogus max_version)?
19:48:45 mordred efried: mutual exclusivity should have a check ... http://git.openstack.org/cgit/openstack/keystoneauth/tree/keystoneauth1/discover.py#n116
19:48:52 mordred efried: but you can certianly make it better!
19:51:44 efried mordred Okay, cool. I haven't grokked the various code paths here, but it looks like that validation won't get run if Discover.version_data is called directly.
19:51:46 openstackgerrit Dean Troyer proposed openstack/cliff master: add tests for display command classes and hooks https://review.openstack.org/481192
19:52:02 efried efried In fact, I just wrote a test case to prove it.
19:52:07 efried Well
19:52:25 efried At least it doesn't trigger for 'max_version': ''
19:52:49 efried which makes sense in light of the loose boolean checks in _normalize_version_args
19:53:10 mordred efried: ah - awesome. well that's definitely a bug then
19:53:35 efried mordred Confirmed even with a real value there.
19:53:38 efried I'll work on it.
19:54:14 mordred efried: thank you for being so thorough!
19:54:41 efried mordred Sure thing; hope I'm not just spinning my wheels. If nothing else, it's helping me understand the code.
19:55:08 mordred efried: you're definitely not spinning your wheels - and you understanding the code is a *GREAT* outcome
20:22:06 efried mordred get_version_data, get_versioned_data, version_data, version_data_for -- Can I get an ugh?
20:23:53 mordred efried: yah- sorry about that
20:44:31 openstackgerrit Monty Taylor proposed openstack/service-types-authority master: Improve validation & documentation https://review.openstack.org/478556
20:51:31 openstackgerrit Merged openstack/service-types-authority master: Improve validation & documentation https://review.openstack.org/478556
21:00:02 openstackgerrit Eric Fried proposed openstack/service-types-authority master: Remove validate.py main() https://review.openstack.org/478562
21:22:20 openstackgerrit Monty Taylor proposed openstack/service-types-authority master: Add ./ to latest link https://review.openstack.org/481283
21:22:27 openstackgerrit Gage Hugo proposed openstack/python-openstackclient master: WIP - Add project tags functionality https://review.openstack.org/481284
21:31:34 openstackgerrit Merged openstack/service-types-authority master: Add ./ to latest link https://review.openstack.org/481283
21:50:04 efried mordred The return from discover.get_version_data - that's allowed to say max_version='latest', right?
21:52:47 efried kinda looks like not.
21:52:49 mordred efried: the top-level get_version_data?
21:52:54 efried yeah
21:53:07 mordred nope - that'll only have values that show up in version discovery documents
21:58:35 openstackgerrit Monty Taylor proposed openstack/service-types-authority master: Stop publishing yaml file https://review.openstack.org/481305
22:19:01 openstackgerrit Eric Fried proposed openstack/keystoneauth master: normalize_version_number([1]) => (1, 0) and docs https://review.openstack.org/481309
22:19:08 efried mordred ^ it's a start.
22:24:16 efried mordred Is there some reason you didn't want Discover.versioned_data_for() to return an EndpointData?
22:24:22 mordred efried: great patch! I left a few comments - mostly centered around one thing that will likely make you punch yourself in the face
22:26:46 mordred efried: I'm not coming up with a good reason right off the bat - it's probably a result of how that stack was developed
22:27:31 efried mordred Possibly you were trying to keep it consistent with data_for(), which it seems to supersede. But it seems like since you're superseding anyway, you might as well make it prettier. I haven't dug all the way in yet, though.
22:27:34 mordred efried: originally versioned_data_for was just an update to data_for
22:27:38 efried right
22:27:44 mordred efried: but then we realized that it was changing data_for's behavior
22:27:53 mordred which we can't do
22:27:58 efried So might as well shoot the whole hog.
22:28:10 mordred so the solution was to just split it out - which we could do without reworking the whole stack
22:28:39 mordred HOWEVER - we're not released yet -so reworking it to return an EndpointData potentially makes sense
22:28:52 mordred (like you say - might was well shoot the whole hog)
22:30:29 mordred efried: might be a smidge tricky given how get_versioned_data works ... but if you see a path - go for it
22:30:35 efried k

Earlier   Later