| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-03-14 | |||
| 14:04:02 | mriedem | we expect everyone to just be using curl? | |
| 14:04:11 | mriedem | bra | |
| 14:04:13 | mriedem | *brb | |
| 14:04:40 | cdent | yes, the spec did say that, but I'm not sure we've actually hit those use cases yet, so might be just as easy to wait? | |
| 14:04:49 | cdent | sick bra, have a great time | |
| 14:05:45 | bauzas | mriedem: yeah, I began to work on providing a separate module for that, but I'd prefer to have a separate client | |
| 14:06:06 | dansmith | mriedem: a client for use in cron may make sense, but not a separate client for server code, IMHO | |
| 14:06:27 | dansmith | mriedem: like a simple thing you can run from the cli to report a particular thing, that's not overly complex, but I would expect cinder to just use requests | |
| 14:06:42 | bauzas | dansmith: well, a python client could be cool for OSC | |
| 14:06:58 | bauzas | and for other projects | |
| 14:07:04 | johnthetubaguy | dansmith: melwitt: macsz: seeing bug patterns emerge around this kind of thing: https://bugs.launchpad.net/nova/+bug/1672624 | |
| 14:07:04 | openstack | Launchpad bug 1672624 in OpenStack Compute (nova) "Ceph volumes attached to local deleted instance could not be correctly handled" [Undecided,New] | |
| 14:07:11 | dansmith | bauzas: for human usage, yes, which I think exists today | |
| 14:07:56 | openstackgerrit | Danfly proposed openstack/nova master: Completely remove mox from unit/network/test_linux_net.py https://review.openstack.org/445504 | |
| 14:08:54 | sdague | bauzas: so, honestly, I really think that clients are an anti-pattern | |
| 14:09:28 | bauzas | sdague: not sure I understand your concerns | |
| 14:09:53 | bauzas | IMHO, we could provide a python placementclient package so OSC could use it for providing CLI | |
| 14:10:08 | sdague | here is the issue with that | |
| 14:10:14 | sdague | and why I think it's an anti pattern | |
| 14:10:16 | bauzas | exactly like the same difference between nova CLI and novaclient python module | |
| 14:10:51 | sdague | if you say that the client code is the REST api, you have documentation and examples for that, which are part of our normal process | |
| 14:11:25 | sdague | if you say that the client is this other library, then you need to duplicate the entire set of documentation specific to the class structure for that. And no longer is your rest api your documentation. | |
| 14:11:48 | sdague | which needs to include all the examples, etc | |
| 14:12:31 | bauzas | well, other projects do that, nope ? | |
| 14:12:47 | bauzas | we have REST API documentation, but we also have CLI and python bindings docs | |
| 14:12:50 | sdague | they don't, which is why OSC has been gutting a bunch of that | |
| 14:12:58 | bauzas | and when I was an operator, it was not a problem for me | |
| 14:13:23 | bauzas | for example, when I was an operator by Diablo, I looked at the novaclient python bindings docs | |
| 14:13:33 | bauzas | well, Essex sorry by that time | |
| 14:13:42 | bauzas | and I wrote my scripts using those | |
| 14:13:56 | bauzas | for bash things I used CLI docs | |
| 14:14:13 | bauzas | and when I wanted to play with the REST API, well, then... actually I never did this :) | |
| 14:14:24 | cdent | sdague: I have never agreed with you more than I agree with this conclusion " And no longer is your rest api your documentation." | |
| 14:14:36 | bauzas | because getting a token is something a bit confusing honestly | |
| 14:14:48 | bauzas | now, I understand how to get a token | |
| 14:14:57 | sdague | bauzas: it is, and that's why we hide that behind keystoneauth1 | |
| 14:15:01 | bauzas | but as an user, I'd prefer to just use a python client for that | |
| 14:15:23 | sdague | but after that point, GET some json is pretty straight forward | |
| 14:15:30 | bauzas | I agree | |
| 14:15:43 | bauzas | if the REST API is well documented :) | |
| 14:15:55 | bauzas | which was not the case previously | |
| 14:15:57 | sdague | bauzas: right, and it would be much better to make that true | |
| 14:16:08 | sdague | then write tons of other equally poor bits of documentation | |
| 14:16:21 | bauzas | sdague: maybe because I'm more a developer than an operator, I looked at how novaclient was working | |
| 14:16:21 | sdague | I am excited that you found it good enough for your use case | |
| 14:16:58 | bauzas | sdague: but it was easier for me to look at the client code to understand the logic than trying to see how the REST API was working | |
| 14:17:01 | sdague | but the experience I've hit with other people is basically why they all use shade now instead of any of the clients | |
| 14:17:21 | sdague | bauzas: the REST api docs were pretty terrible at that time | |
| 14:17:28 | bauzas | that said, it's now 4 years later, and we're in a far better situation for the REST API thanks to you and other API people :) | |
| 14:17:34 | bauzas | yeah zactly | |
| 14:17:51 | sdague | bauzas: so, please don't use the lessons from 5 years ago about the right patterns for today | |
| 14:17:58 | bauzas | fair enough | |
| 14:18:29 | bauzas | but the scheduler.report client module is doing more than just GET and POST, right? | |
| 14:19:16 | bauzas | should we leave other service projects writing their own methods like we do in that current scheduler module ? | |
| 14:19:42 | macsz | johnthetubaguy: thanks, will take a look as soon as I get a break from the training | |
| 14:21:27 | sdague | bauzas: the things that seem to be common is pretty much just the safe_connect wrapper there, what bits of complex logic would you expect would be needed in neutron? | |
| 14:22:12 | dansmith | no, I think a lot of what is there is pretty nova-specific | |
| 14:22:31 | sdague | bauzas: I'd much rather start under the assumption that a library is not needed, walk through doing an implementation in another service, figure out if there are common things that would be handy to share (like the safe connect bit) and maybe put that into a utility mixin or back in keystoneauth | |
| 14:22:40 | dansmith | there may be a few re-usable things, like "ensure_resource_provider()" but that's really just a get-and-post-on-404 | |
| 14:22:41 | dansmith | which is not worth a separate client | |
| 14:22:46 | bauzas | sdague: this for example https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L484 | |
| 14:23:09 | bauzas | neutron would provide their inventories, right? | |
| 14:23:12 | bauzas | or cinder | |
| 14:24:18 | cdent | (safe connect in keystoneauth makes good sense) | |
| 14:24:19 | sdague | bauzas: maybe, how about implement that first in one of those services and see | |
| 14:24:48 | openstackgerrit | Danil Akhmetov proposed openstack/nova master: Use proper user and tenant in the owner section of libvirt.xml. https://review.openstack.org/399679 | |
| 14:25:01 | bauzas | sdague: yeah we could possibly wait for other service projects calling the placement API and see if we need a common client package, maybe | |
| 14:25:03 | dansmith | bauzas: it's long, but really not much meat in there.. retry on 409 | |
| 14:25:14 | cdent | bauzas: neutron built in their own custom client for inventories: https://review.openstack.org/#/c/358658/ | |
| 14:26:07 | bauzas | cdent: but they don't have our story yet | |
| 14:26:16 | bauzas | sorry | |
| 14:26:20 | bauzas | s/story/background | |
| 14:26:33 | cdent | meaning what? | |
| 14:26:35 | bauzas | so I'm afraid of them missing some corner case | |
| 14:26:47 | bauzas | that we could have fixed in the scheduler.report module | |
| 14:26:55 | bauzas | like https://github.com/openstack/nova/blob/master/nova/scheduler/client/report.py#L596 | |
| 14:27:37 | bauzas | anyway, I'm not *that* concerned | |
| 14:27:54 | bauzas | if there is no traction around a specific common client, fine by me | |
| 14:28:00 | sdague | bauzas: honestly, I feel like this is part of the anti pattern. Deciding that projects that consume other project apis don't need to know how to use those APIs, and we'll hide everything in a library. If the API is hard to use correctly, that's data, and we should take it back to the API, not put a bunch of work around code around it | |
| 14:28:10 | bauzas | let's wait other projects using the placement API, and we'll see if we need a common one | |
| 14:28:52 | sdague | I mean, we were seriously talking 2 cycles ago about the neutron team building a nova consumption interface on top of their neutron client, because it was too hard to use. | |
| 14:28:52 | bauzas | sdague: that's a reasonable concern | |
| 14:29:02 | sdague | maybe it was more than 2 cycles ago | |
| 14:29:24 | bauzas | :) | |
| 14:29:53 | bauzas | yeah, that's a trade-off, I agree | |
| 14:30:09 | bauzas | if we can keep things simple, I'm all good | |
| 14:30:31 | bauzas | again, I'm seeing myself in a minority on that, so fair enough | |
| 14:30:37 | mriedem | i'm back, caught up | |
| 14:30:45 | jaypipes | mriedem: I'm strongly opposed to a separate client for placement. Having an openstackclient module for resource providers and inventories/allocations/aggregate associations is A-OK, but not a separate python client. | |
| 14:30:50 | mriedem | so no to the client for APIs, and no to CLI for osc? | |
| 14:30:57 | bauzas | it's a valid concern, and if people don't really want to have that common client, plus the concerns you mention, then I'm okay | |
| 14:31:27 | mriedem | i'm good with not having a python API client | |
| 14:31:37 | bauzas | jaypipes: OSC being only a CLI wrapper ? | |
| 14:31:38 | mriedem | i was wondering more about CLI | |
| 14:31:48 | mriedem | osc is a cli wrapper, not an api library | |
| 14:31:54 | mriedem | it uses the other client API libraries and the sdk | |
| 14:31:57 | bauzas | yeah, just wanted to reassure that | |
| 14:32:08 | bauzas | that we won't push smart code in OSC | |
| 14:32:40 | bauzas | again, if people feel confident with that, let's stick with dansmith's plan given in the ML | |
| 14:32:53 | bauzas | I don't want to divert too much on that | |