| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2017-10-03 | |||
| 13:42:02 | bauzas | cdent: just a slight concern about possibly overthinking about times with https://review.openstack.org/#/c/496853/3 | |
| 13:42:52 | cdent | bauzas: your etag fear has been noted on your api merit badge worksheet as a demerit | |
| 13:43:37 | bauzas | hah | |
| 13:43:44 | jaypipes | lol | |
| 13:44:08 | bauzas | honestly, I just want to make sure we have a very small implementation for that | |
| 13:44:10 | cdent | bauzas: on the last-modified time: since the time info is already there, seems like we may as well use it, because the spec says we should (if possible) include the _real_ time of update | |
| 13:44:20 | bauzas | sure, I understand that | |
| 13:44:32 | bauzas | but IMHO, keeping it simple for Queens isn't bad | |
| 13:44:42 | cdent | since we dismissed etags as part of placement long ago, I think we should stick with that plan, I only include the option to be complete | |
| 13:44:44 | bauzas | unless you really wanna cache | |
| 13:44:56 | bauzas | and then, we should possibly use etags | |
| 13:45:19 | bauzas | so, my thoughts are : do the very small change and just use the current time, or use Etags :p | |
| 13:45:50 | cdent | I think the last-modified time is useful metadata for generic users of the placement service, maybe not in nova-scheduler, but for random unknowns. And since I’ve alrady got a working implementation, it’s not too much effort to finish it | |
| 13:46:22 | bauzas | cdent: well, it needs then to leak out the DB details to the object | |
| 13:46:32 | bauzas | we do that for a lot of stuff of course | |
| 13:46:35 | cdent | one sec | |
| 13:46:51 | bauzas | but I thought our placement objects shouldn't be using that | |
| 13:47:03 | bauzas | anyway, I don't want to nitpick over it | |
| 13:47:04 | cdent | bauzas: this is the wip, is not hard: https://review.openstack.org/#/c/495380/7/nova/objects/resource_provider.py | |
| 13:47:16 | cdent | we alraedy have those fields | |
| 13:47:20 | bauzas | yeah I know | |
| 13:47:27 | bauzas | using a mixin isn't hard | |
| 13:47:44 | bauzas | it's just we're exposing those DB details out to the API | |
| 13:48:15 | bauzas | cdent: is it the first OpenStack project doing that ? (please use your API SIG hat :p ) | |
| 13:48:57 | cdent | I don’t understand what you mean by “exposing those db details out the to the api”. You mean last modified time? Why is that a problem? | |
| 13:49:06 | sdague | mriedem: ok, I'm really struggling about why on - https://review.openstack.org/#/c/501017/2/specs/queens/approved/flavor-description.rst | |
| 13:49:15 | sdague | because that's really already there with flavor name | |
| 13:49:42 | jaypipes | edleafe, bauzas, cdent, stephenfin, dansmith, mriedem: I think the only thing remaining on https://review.openstack.org/#/c/497713/10/specs/queens/approved/add-trait-support-in-allocation-candidates.rst is to agree on the name of the parameter. The options are traits=, required=, required_traits=. Let's just pick one. Please #vote for your first and second pick. I'll go first. | |
| 13:50:01 | jaypipes | #vote 1. required=, 2. traits= | |
| 13:50:01 | bauzas | I need to review that spec | |
| 13:50:13 | dansmith | #vote 1. required 2. required= 3. requires= | |
| 13:50:19 | jaypipes | lol | |
| 13:50:25 | dansmith | jaypipes: I thought everyone was okay with required=? | |
| 13:50:40 | jaypipes | dansmith: I don't believe mriedem was and I know edleafe wasn't. | |
| 13:50:42 | edleafe | required= or traits= are fine with me | |
| 13:50:50 | dansmith | edleafe: said he was | |
| 13:50:51 | cdent | bauzas In my limited review of openstack apis, there’s a mix of who does and does not use last-modifed. If we’re looking to limit the amount of work in Queens, we could choose not to do this spec, it isn’t really required in any way. | |
| 13:50:52 | edleafe | requires= is definitely not | |
| 13:50:54 | jaypipes | or was it requires=... | |
| 13:50:55 | bauzas | required= for me | |
| 13:50:56 | jaypipes | yeah, sorry | |
| 13:51:05 | stephenfin | #vote 1. traits=, 2. required= | |
| 13:51:06 | cdent | mriedem expresed dislike for required, yes? | |
| 13:51:08 | dansmith | jaypipes: I think mriedem said required= was okay too | |
| 13:51:13 | edleafe | verbs don't work | |
| 13:51:15 | bauzas | because we could implement preferred= later | |
| 13:51:18 | jaypipes | understood. | |
| 13:51:27 | dansmith | bauzas: exactly | |
| 13:51:31 | stephenfin | ahhh | |
| 13:51:46 | edleafe | preferred won't be in placement, right? | |
| 13:51:52 | edleafe | that's a weigher issue | |
| 13:51:55 | openstackgerrit | John Garbutt proposed openstack/nova master: Re-use existing ComputeNode on ironic rebalance https://review.openstack.org/508555 | |
| 13:52:01 | cdent | #vote 1. required 2. required_traits | |
| 13:52:03 | mriedem | i said i cared less about required= even though i didn't like it | |
| 13:52:05 | bauzas | edleafe: I'm not advocating for it now | |
| 13:52:08 | mriedem | i was -1 on the ?required='' thing | |
| 13:52:15 | bauzas | edleafe: I'm just saying it *could* be possible | |
| 13:52:17 | dansmith | edleafe: I don't think so, we still have to pull out things that might have that over things that don't at all, right? | |
| 13:52:20 | jaypipes | edleafe: no plans *currently*, but we still would likely need a corresponding parameter for preferred to pass to the scheduler of course. | |
| 13:52:23 | cdent | yeah, I fixed the required=‘’ thing, thank goodness | |
| 13:52:37 | mriedem | sdague: i can't say how often people are changing flavor names randomly | |
| 13:52:55 | edleafe | jaypipes: ok, I thought we were just thinking of the placement api | |
| 13:52:56 | mriedem | there were a few operators that said they would use this on the spec | |
| 13:53:08 | jaypipes | mriedem: what's your vote then? traits=? | |
| 13:53:11 | sdague | mriedem: sure, so let's just make name mutable | |
| 13:53:13 | bauzas | cdent: honestly, like I said, I don't want to take too much time on that spec, +Wd :) | |
| 13:53:34 | sdague | mriedem: I guess, it feels really weird to add a **3rd** 255 character string to flavor | |
| 13:53:37 | mriedem | sdague: from a ux perspective i don't think name is the thing you want to have 255 characters of detail in | |
| 13:53:52 | sdague | mriedem: because it's called name? | |
| 13:53:58 | mriedem | we have server name and description | |
| 13:53:58 | mriedem | yes | |
| 13:54:05 | mriedem | i don't expect the name of a resource to be super detailed | |
| 13:54:25 | mriedem | jaypipes: huh? i can't handle 3 conversations at once plus reviews right now. | |
| 13:54:27 | sdague | mriedem: we do, but servers' don't also have a server_id field that's a 255 character string | |
| 13:54:36 | mriedem | jaypipes: i thought agreement was on required=? | |
| 13:54:42 | mriedem | since we could have preferred later | |
| 13:54:51 | jaypipes | mriedem: got it. ok, it's settled then. | |
| 13:54:59 | jaypipes | I just wanted to hold a final vote. | |
| 13:55:06 | mriedem | sdague: i don't know why flavor is the weirdo resource with a 255 char id field either | |
| 13:55:19 | sdague | mriedem: because flavor_id == server.name | |
| 13:55:25 | mriedem | but id and name are generally short things | |
| 13:55:26 | sdague | and flavor.name == server.description | |
| 13:55:42 | sdague | or, at least should be treated as such | |
| 13:56:10 | mriedem | how many clouds are filling out the full flavor.name to express everything in that field today? | |
| 13:56:18 | mriedem | including details about baremetal and extra specs | |
| 13:57:09 | sdague | mriedem: don't know, but if that's the concern I'd make the microversion start returning name as a description field instead, and make it mutable after that point | |
| 13:57:39 | mriedem | i'm not going to do that | |
| 13:57:45 | mriedem | we also embed the flavor name in the instance details now, | |
| 13:58:00 | mriedem | so if name starts becoming big ass description, then your output in things like nova show are going to bloat up | |
| 13:58:25 | sdague | mriedem: but that can already be an issue today | |
| 13:59:07 | sdague | if you put a 64 char constraint on id and name at the same time, I'd be fine with it. But I think choose your own adventure on 3 255 character fields is going to lead to more confusion | |
| 13:59:13 | efried | cdent Nice, thanks. | |
| 13:59:40 | bauzas | jaypipes: question in https://review.openstack.org/#/c/497713/10 | |
| 13:59:46 | mriedem | sdague: i don't see how it's confusing, | |
| 13:59:48 | mriedem | name is the name, | |
| 13:59:50 | mriedem | id is a uuid by default | |
| 13:59:59 | mriedem | description is a description, name and description are different things | |
| 14:00:01 | mriedem | in english | |
| 14:00:21 | bauzas | jaypipes: tl;dr say I have a host with 2 PFs, and only one tagged with a trait | |
| 14:00:29 | sdague | but your concern was also bloat because 255 characters should not be used | |