Earlier  
Later
New Search
Posted Nick Remark
#openstack-sdks - 2017-07-06
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
15:34:11 mordred in fact, that is a glance behavior that does not match the docs
15:34:21 GreenBlood Oh
15:35:34 mordred so - fwiw, the way to get shade to show you shared-but-not-accepted images is to do search_images(filters=dict(member_status='all')) ... that's not super friendly - so I think we should add a flag to the api to make it easy to opt-in to seeing _ALL_ images including unaccepted shared images
15:41:36 GreenBlood Good to know
15:42:01 GreenBlood But it's really bugging me this listing issue thingy
15:42:32 GreenBlood that would be really wierd to have shared images on v1 and not v2 no ?
15:48:41 GreenBlood Using "image_api_version: 1" in the cloud config file solves the issue
15:53:22 GreenBlood btw this tool, Shade, is really awesome
15:53:39 GreenBlood I discovered it the other day and I'm amazed
15:56:15 mordred GreenBlood: yay! I'm glad you like it!
15:56:39 mordred GreenBlood: yah - it was a behavior change in the v2 api - v1 doesn't understand shared at all, so it doesn't filter anything
15:57:06 GreenBlood Yeah but search with member_status='all' doesnt change the result I think
15:57:14 GreenBlood or maybe i'm just blind
16:07:14 GreenBlood which would be sad, really
16:21:00 GreenBlood Even with the CLI, doing "glance --os-image-api-version 1 image list" and without specifying the version, the shared images appears or disappears. I'll send a mail to my cloud provider :)
16:21:07 GreenBlood Thank you -again- for your help
16:38:29 openstackgerrit Eric Fried proposed openstack/keystoneauth master: normalize_version_number([1]) => (1, 0) and docs https://review.openstack.org/481309
16:38:49 fried_rice mordred ^
16:59:03 openstackgerrit Merged openstack/keystoneauth master: Switch from oslosphinx to openstackdocstheme https://review.openstack.org/479191
17:12:16 mordred GreenBlood: https://review.openstack.org/481702 made a patch to add a show_all flag

Earlier   Later