Earlier  
Later
New Search
Posted Nick Remark
#openstack-sdks - 2017-07-06
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
22:32:32 openstackgerrit Dean Troyer proposed openstack/cliff master: add tests for display command classes and hooks https://review.openstack.org/481192
22:40:04 openstackgerrit Rajath Agasthya proposed openstack/cliff master: Run hooks for DisplayCommandBase https://review.openstack.org/479157
22:40:05 openstackgerrit Rajath Agasthya proposed openstack/cliff master: add tests for display command classes and hooks https://review.openstack.org/481192
#openstack-sdks - 2017-07-07
01:49:26 dtroyer mordred: I am queueing up some lib releases (will be out of town for a few days), https://review.openstack.org/481355 is for o-c-c. +/-1 it as needed, I don't think there is a rush (yet).
02:58:41 openstackgerrit Merged openstack/cliff master: Run hooks for DisplayCommandBase https://review.openstack.org/479157
03:06:43 openstackgerrit Merged openstack/cliff master: add tests for display command classes and hooks https://review.openstack.org/481192
04:02:37 openstackgerrit lihaijing proposed openstack/python-openstackclient master: Replace six.iteritems() with .items() https://review.openstack.org/481407
06:07:46 openstackgerrit lihaijing proposed openstack/python-openstackclient master: Replace six.iteritems() with .items() https://review.openstack.org/481407
12:40:46 openstackgerrit M V P Nitesh proposed openstack/python-openstackclient master: List images with --tags for 'openstack image list' https://review.openstack.org/481615
13:51:37 openstackgerrit Merged openstack/python-openstackclient master: Use openstackdocstheme in release note https://review.openstack.org/481173
15:00:08 GreenBlood Somebody using the Shade lib/client able to confirm that listing the images omits the shared ones ?
15:04:59 GreenBlood Listing images with glanceclient.images.list() and with the Shade wrapper does not give me the same results it seems
15:10:28 GreenBlood (Altho judging by the API calls they look very much the same so I don't understand). There is only "is_public=None" and "limit=20" in the glanceclient call, and no argument on the Shade call ([endpoint]/v2/images)
15:24:09 mordred GreenBlood: there is a potential behavioral difference depending on how new the glance service is
15:24:55 mordred GreenBlood: gimme a few minutes and I'll go dig up what the deal is
15:25:10 mordred actually - that's a silly thing to say
15:26:45 GreenBlood Yeah but the service is the same wether I use the glanceclient or shade/openstackclient nah ?
15:27:40 GreenBlood Oh wait I think I noticed something
15:27:54 GreenBlood I think our glanceclient uses v1 and shade uses v2
15:28:08 GreenBlood that would make sense
15:29:02 mordred ah - that'll definitely do it - however, it's possible also that we're not sending somehting we need to to get shared images returned
15:29:58 GreenBlood Judging by the HTTP debug, shade sends "GET /v2/images " and our code using glanceclient sends "GET /v1/images/detail?limit=20&is_public=None"
15:31:30 mordred kk. so - you can force shade to use v1 by setting image_api_version=1 in your clouds.yaml file - also - for v2, there is this note:
15:31:44 mordred "If you omit this parameter (visibility ), the response shows public, private, and those shared images with a member status of accepted."
15:32:11 mordred so images in v2 default to "shared" if you don't set any other status, and 'shared' images will only show up for a user in v2 by default if that user has "accepted" the image
15:33:21 mordred GreenBlood: so - list_images in v2 with no other parameters omitting shared by not accepted images is 'correct' behavior - although it might be surprising
15:33:35 GreenBlood Yeah well the user has accepted, I can see it in Horizon and stuff.
15:33:47 GreenBlood I'll try using v1 api but yeah its a bit wierd imho
15:33:50 mordred ah! well, that seems like a whole different bug

Earlier   Later