Earlier  
Later
New Search
Posted Nick Remark
#openstack-sdks - 2018-03-20
15:50:51 dtroyer there is a parallel to this with security groups and security group rules
15:51:08 mnaser so what was the solution from that parallel
15:51:54 dtroyer security group rules _are_ a top-level resource and treated as that everywhere.
15:52:29 dtroyer they have their own properties/attributes and require the full set of create/delete/list/show commands
15:52:38 dtroyer the member list isn't the same thing
15:52:53 dtroyer so that's not the best example of a one-to-many relationship after all :(
15:54:10 dtroyer the project list attached to an image is just another property that can have multiple values
15:55:46 mnaser dtroyer: well, its an http resource so its not like it comes along a 'image show' http request
15:56:47 dtroyer right, but frankly I don't care what the REST API looks like, it is about the model presented to the user. which is one reason not automatically pulling the data is a consideration, via adding an option to image show
15:57:18 mnaser i mean if it was a flat list (ex: projects only), it wouldn't be that bad to add it to the show
15:57:31 mnaser the issue is that it presents other things like .. member id and status (ex: accepted, pending, etc)
15:57:53 mnaser i dont mind pushing it up to show but the thing that would make me sad is that the user will have a very hard time parsing it using the tools available in osc
15:58:03 mnaser ex cant use --columns and so on
16:00:14 dtroyer right, that's a general problem with a number of our data structures, and is one reason we played with adding a jq interface to make dealing with nested data via JSON easier. turns out to still be rather messy to do in a CLI
16:01:34 openstackgerrit Jens Harbott (frickler) proposed openstack/keystoneauth master: Be more verbose when version discovery fails https://review.openstack.org/554044
16:02:48 dtroyer mnaser: so since image memebr has an ID, that improves the case for it being a top-level resource. however, I'm not aware of any place to actually use that ID. It is the status that makes this messy.
16:06:22 dtroyer I still don't like "image member" but it is better than "image project". unless we come up with something obviously better, lets add "image member" as a top-level resource, which means adding it to the resource list in doc/source/cli/commands.rst. It should be in its own module but I don't plan on adding other commands so leaving it in image.py is probably fine.
16:07:41 dtroyer I don't know what the API looks like offhand, but adding a —project option to image member list might be nice at some point to only list images that a particular project has access to. future.
16:08:40 dtroyer and now lunch. I'll summarize this in the review after lunch. thanks for the command and for letting me think through this out loud to refresh my mind.
16:09:03 dtroyer EmilienM: I'll get to your review after lunch too
16:10:29 mordred dtroyer, mnaser: ok - back ...
16:10:50 EmilienM dtroyer: thanks!
16:15:50 openstackgerrit Monty Taylor proposed openstack/python-openstacksdk master: Provide OpenStackConfigException backwards compat https://review.openstack.org/554615
16:16:32 mordred mnaser, dtroyer: ^^ that should unstick mnaser's issue ... although now that we can make osc-lib depend on openstacksdk>=0.12 - we could make a pass through and update it to remove the occ paths... I'll do that as a followup
16:17:36 mordred perhaps land that patch, cut an sdk point release, bump the min in global-requirements, then push up osc-lib patches to remove occ fallback paths?
16:17:44 mordred dtroyer: that sound like an ok plan?
16:37:57 mordred frickler: left a comment on your ksa patch
16:40:42 frickler mordred: I tend to agree with your comment, would you want to reword the original message down on L199 accordingly? would seem sensible to keep those in sync
16:49:06 mordred frickler: I'd love to - it's my least favorite error message ... however, ...
16:49:21 mordred kmalloc: ^^ do we consider error message text part of keystoneauth's api?
16:49:31 kmalloc hm
16:49:45 kmalloc mordred: i hope we don't. but honestly, haven't thought about that
16:49:53 kmalloc mordred: maybe we should?
16:49:55 mordred kmalloc: me either
16:50:09 kmalloc i'm willing to make a call on that right here conferring with you ... and....
16:50:14 kmalloc i'm inclined to say no
16:50:19 kmalloc but the exception class would be.
16:50:25 mordred kmalloc: specifically, "Could not determine a suitable URL or the plugin" - which almost never actually means what it says
16:50:33 kmalloc yeah
16:50:37 mordred kmalloc: yah - exception class == part of aPI for sure
16:51:10 kmalloc so, i think the message shouldn't be because it might change to be "better" in the future
16:51:20 kmalloc unless it's structured data.
16:51:30 kmalloc whichcase it would be part of the api
16:51:47 kmalloc if it's just a "message", it's not part of the api (in my mind, but i could be convinced otherwise)
16:51:49 mordred kmalloc: yah - but in that case I'd expect us to put that data on the execeptoin instance
16:51:52 kmalloc right
16:52:05 kmalloc so, you're good with saying it's not part of the API?
16:52:17 mordred kmalloc: so maybe let's say "no" - and if people are finding places in which they need to look at exception text, then we nee more specific exception classes
16:52:23 kmalloc wfm
16:52:25 kmalloc ^5
16:52:34 mordred frickler: ^^
16:52:46 kmalloc exception text is useful for humans
16:52:51 kmalloc not for programatic use.
16:52:56 openstackgerrit Merged openstack/python-openstacksdk master: Fix 'block_store' aliases define error https://review.openstack.org/554403
16:53:48 mordred frickler: also - fwiw, maybe toss in an 'if isinstance(e, exceptions.DiscoveryFailure): raise else: raise exceptions.DiscoveryFailure(...) so that we don't lose the original discovery context just because we made an attempt to guess a versioned endpoint
17:28:16 openstackgerrit Chris Dent proposed openstack/microversion-parse master: Improve Version.matches() method https://review.openstack.org/550251
17:28:17 openstackgerrit Chris Dent proposed openstack/microversion-parse master: Allow passing a json_error_formatter to the middleware https://review.openstack.org/550263
17:28:17 openstackgerrit Chris Dent proposed openstack/microversion-parse master: Add MicroversionMiddleware https://review.openstack.org/550252
17:28:18 openstackgerrit Chris Dent proposed openstack/microversion-parse master: Do not use redundant attr check for keys in fold_headers https://review.openstack.org/554547
17:28:18 openstackgerrit Chris Dent proposed openstack/microversion-parse master: Clean up package version and python version support https://review.openstack.org/550264
20:07:17 openstackgerrit Merged openstack/python-openstacksdk master: Use defined version instead of service.version parameter https://review.openstack.org/554223
21:06:11 openstackgerrit Dean Troyer proposed openstack/python-openstackclient master: Fix additional output encoding issues https://review.openstack.org/554698
21:55:13 dhellmann dtroyer : regarding https://review.openstack.org/#/c/554698/1 I wonder if cliff should grow support for progress bars?
21:56:23 dhellmann https://pypi.python.org/pypi/progressbar was pretty easy to use in downpour
21:57:03 dhellmann oh, although I wonder if that's actively supported any more if it's on code.google.com
21:57:16 dhellmann ah, redirects to https://github.com/niltonvolpato/python-progressbar
21:59:18 dhellmann aha, I actually used https://pypi.python.org/pypi/progressbar2
21:59:56 dhellmann anyway, let me know if you think that would be a useful thing to be able to do via cliff
22:00:11 dtroyer dhellmann: maybe? I really didn't know what to do with that _show_progress() function. Putting it where I did made calling easy to get cliff's stdout but made testing harder than I wanted to finesse to get a ppinter to the nested function.
22:00:33 dhellmann yeah
22:01:08 dhellmann oh, you could have made it a method instead of a closure
22:01:24 dhellmann that would make it easier to mock it out or whatever
22:01:39 dtroyer but then it would require passing self around and changing the signature of the callback
22:01:53 dhellmann self is passed automatically to a bound method
22:02:14 dhellmann if the signature is "def _show_progress(self, progress)" and you pass self._show_progress the callback will expect 1 arg
22:02:21 dtroyer ok, so I didn't even try that because I assumed it needed a second argument.
22:02:30 dhellmann nah, it does the easy thing
22:02:36 dtroyer that is certainly a better solution
22:02:40 dhellmann what sorts of values are passed to that method?
22:02:59 dhellmann are they text like "working" or are they percentages?
22:05:02 dtroyer it is anything… if the resource being waited on has a progrss attribute, that is passed back
22:05:13 dtroyer *progress attribute
22:05:14 dhellmann so a progress bar might not really work
22:06:41 dhellmann oh, there's an "animated marker"
22:07:56 dhellmann eh, it's something to think about
22:08:58 dtroyer it isn't something I really thing we should be doing, as that kind of stuff often just pollutes logs, but some like it as a touchy-feedbacky thing, even if it is usually not very accurate
22:09:32 dhellmann that's true, i guess
22:09:50 dtroyer it could be tied to logging levels to address that, but I've never really cared enough to follow up
22:10:00 dhellmann I guess it's hard to tell if self.stdout is sys.stdout after we wrap it up
22:10:15 dhellmann it could be its own flag --progress or something
22:10:42 dhellmann I don't feel strongly enough about it to push and implement it; it just seemed like a thing to talk about in light of that patch
22:11:07 dhellmann I'll leave a wishlist ticket in lp in case someone else wants to do it
22:11:34 dtroyer sure, and I appreciate the reviews, this (encoding I/O) is an area I don't seem to be able to keep in my head and have to periodically e-learn it
22:12:20 dhellmann it's like a sandwich. the encoding/decoding steps are the bread and the application is the peanut butter
22:13:00 dtroyer :) it much be dinnertime in GA?
22:13:47 openstack Launchpad bug 1757278 in cliff "add support for progress bar/widgets" [Wishlist,New]
22:13:47 dhellmann https://bugs.launchpad.net/python-cliff/+bug/1757278
22:14:02 dhellmann haha, yeah, I think I'm hungry :-)
22:14:42 dhellmann speaking of eating, it' time to go exercise some
23:44:01 mordred dtroyer, dhellmann: fwiw, ianw has poked a little bit at the idea of having some of the calls in shade (so now obviously for sdk) to be able to have progress callbacks...

Earlier   Later