| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2020-04-09 | |||
| 10:51:46 | sean-k-mooney | so i think it wont normalise on your behalf | |
| 10:51:52 | bauzas | you could do whatever you want, but ask placement the right things | |
| 10:51:53 | sean-k-mooney | it will reject the creation fo the trait? | |
| 10:52:04 | bauzas | sean-k-mooney: that's the whole point, I dunno | |
| 10:52:26 | sean-k-mooney | ya i have never tried to check honestly | |
| 10:52:31 | bauzas | I dunno whether Placement API is strict with naming or just allows anything to come by and normalize it | |
| 10:52:44 | bauzas | with the latter, I have concerns | |
| 10:52:47 | bauzas | this is not RESTful | |
| 10:53:11 | bauzas | if I'm creating an object, I'm expecting the output of the creation is identical to the object I passed | |
| 10:53:21 | bauzas | from a semantics perspective | |
| 10:53:28 | bauzas | (or idempotent if you prefer) | |
| 10:55:27 | sean-k-mooney | they dont actully have a test case for it but from the doc text i would expect you will get a 400 | |
| 10:55:29 | sean-k-mooney | https://github.com/openstack/placement/blob/master/placement/tests/functional/gabbits/traits.yaml#L17-L34 | |
| 10:56:41 | openstackgerrit | Sylvain Bauza proposed openstack/nova master: Functional test with pGPUs https://review.opendev.org/717975 | |
| 10:56:47 | bauzas | sean-k-mooney: FWIW, the docs I wrote for vgpu multiple types only mentions trait names being capitalized and not having other chars but the ones wanted https://8ae97f93626416bcf134-be430d2f919a8698d2e96141ed3ac146.ssl.cf1.rackcdn.com/715490/10/check/openstack-tox-docs/b670504/docs/admin/virtual-gpu.html#optional-provide-custom-traits-for-multiple-gpu-types | |
| 10:57:09 | kashyap | sean-k-mooney: Yeap; noted - on suspend. Thanks | |
| 10:57:56 | bauzas | sean-k-mooney: yeah thanks for the gabbit, that proves the Placement API ref should be updated to mention such limitations | |
| 10:58:31 | sean-k-mooney | well gabbit should also be extened to test an invalid trait that is not normalised | |
| 10:58:51 | bauzas | sean-k-mooney: talking of https://docs.openstack.org/api-ref/placement/?expanded=update-resource-provider-traits-detail#update-resource-provider-traits | |
| 10:59:50 | bauzas | whoops, wrong link | |
| 11:00:19 | bauzas | sean-k-mooney: this one sorry https://docs.openstack.org/api-ref/placement/?expanded=update-traits-detail#traits | |
| 11:00:49 | bauzas | Error response codes: badRequest(400) 400 BadRequest if trait name is not prefixed with CUSTOM_ prefix. | |
| 11:00:54 | sean-k-mooney | bauzas: found it https://github.com/openstack/placement/blob/master/placement/schemas/common.py#L17-L22 | |
| 11:01:13 | bauzas | I wasn't crazy \o/ | |
| 11:01:15 | sean-k-mooney | they are using json schema for that validation https://github.com/openstack/placement/blob/master/placement/schemas/common.py#L17-L22 | |
| 11:01:17 | bauzas | stephenfin: ^ | |
| 11:01:43 | sean-k-mooney | with this scema https://github.com/openstack/placement/blob/master/placement/schemas/trait.py | |
| 11:01:44 | bauzas | I was betting this morning that it *wasn't* possible to create traits with hyphens in them | |
| 11:01:53 | stephenfin | ||
| 11:01:53 | stephenfin | <stephenfin> Sec, lemme drag it out | |
| 11:02:17 | stephenfin | With 1.33 they become alphanumeric, '_', and '-' with a length limit of 64. | |
| 11:02:51 | bauzas | you're talking of the group info | |
| 11:02:55 | bauzas | not the trait | |
| 11:03:13 | bauzas | https://github.com/openstack/placement/blob/master/placement/schemas/common.py#L20 | |
| 11:03:20 | sean-k-mooney | stephenfin: this is the important bit https://github.com/openstack/placement/blob/master/placement/schemas/common.py#L17-L22 | |
| 11:04:12 | sean-k-mooney | so it expands to " "^CUSTOM_[A-Z0-9_]+$" | |
| 11:04:38 | stephenfin | I'm confused. Isn't that exactly what I have? | |
| 11:04:40 | sean-k-mooney | so hyphen is not allowed in the name | |
| 11:04:47 | stephenfin | https://review.opendev.org/#/c/718461/4/nova/api/validation/extra_specs/traits.py@65 | |
| 11:04:59 | stephenfin | I don't allow hyphens in that trait name. I only allow them in the group | |
| 11:05:12 | sean-k-mooney | no | |
| 11:05:24 | sean-k-mooney | [a-zA-Z0-9_-] | |
| 11:05:28 | sean-k-mooney | the trailing - | |
| 11:05:32 | sean-k-mooney | allows hypenes | |
| 11:05:35 | stephenfin | that's the group name, not the trait name | |
| 11:05:42 | sean-k-mooney | oh sorry group | |
| 11:05:47 | stephenfin | trait{group}:CUSTOM_{trait} | |
| 11:05:53 | bauzas | stephenfin: tbc, I clarified https://review.opendev.org/#/c/718461/4 | |
| 11:05:56 | stephenfin | r'([a-zA-Z0-9_-]{1,64})?' applies to group | |
| 11:05:58 | bauzas | (my -1) | |
| 11:05:59 | sean-k-mooney | yep what you have is correct | |
| 11:06:04 | stephenfin | r'[A-Z0-9_]+' applies to trait | |
| 11:06:13 | bauzas | my only concern is with traits | |
| 11:06:27 | sean-k-mooney | bauzas: line 65 handels that | |
| 11:06:35 | sean-k-mooney | and that is correct | |
| 11:06:36 | bauzas | of what ? | |
| 11:06:47 | sean-k-mooney | https://review.opendev.org/#/c/718461/4/nova/api/validation/extra_specs/traits.py@65 | |
| 11:07:08 | sean-k-mooney | stephenfin: although i think bauzas was originally not talking about your validation stuff | |
| 11:07:16 | stephenfin | bauzas: replied | |
| 11:07:23 | bauzas | yeah, again my concern is just abotu https://review.opendev.org/#/c/718461/4/nova/tests/unit/api/validation/extra_specs/test_validators.py@33 | |
| 11:07:32 | openstackgerrit | Balazs Gibizer proposed openstack/python-novaclient master: Microversion 2.87 - Stable device boot from volume rescue https://review.opendev.org/714956 | |
| 11:07:32 | openstackgerrit | Balazs Gibizer proposed openstack/python-novaclient master: Microversion 2.86 - Extra spec validation https://review.opendev.org/718057 | |
| 11:07:36 | stephenfin | I think you're mixing up the {group} and {trait} parts of the validator | |
| 11:07:42 | bauzas | oh man | |
| 11:07:48 | bauzas | I'm getting headache | |
| 11:08:31 | sean-k-mooney | bauzas that is the list of namespaces | |
| 11:08:38 | sean-k-mooney | so the partern ther is the group name | |
| 11:08:40 | bauzas | yup, turned up my vote | |
| 11:08:47 | bauzas | anyway, lunch | |
| 11:08:50 | sean-k-mooney | stephenfin: could you use constnts for this | |
| 11:09:05 | sean-k-mooney | so in the test you can just refer to the constnat | |
| 11:09:25 | sean-k-mooney | that might make it clearer? | |
| 11:09:37 | sean-k-mooney | im not going to -1 over that but it might help | |
| 11:09:41 | stephenfin | Not really, because we "build" the regex so I have proper groups | |
| 11:10:04 | stephenfin | Also, I'm always reluctant to use constants in unit tests, because if you've a mistake in the constant then you propagate the mistake | |
| 11:10:49 | sean-k-mooney | ya that is a factor although i have seen many typos in unit test where the typo was not important | |
| 11:11:00 | sean-k-mooney | untill you change something | |
| 11:11:23 | sean-k-mooney | ok im going to go re review the vgpu stuff | |
| 11:12:24 | sean-k-mooney | oh lyarwood changes are on top of yours i was wondering why the validation serires was suddenly much longer | |
| 11:14:48 | lyarwood | yeah microversion fun | |
| 11:15:18 | gibi | bauzas: after your lunch, could you check and upgrade your vote on https://review.opendev.org/#/c/700812 ? | |
| 11:16:31 | sean-k-mooney | lyarwood: its still not as bad as last cycle when we were trying to land numa live migration, pcpus in plamcnet and vpmem all at once | |
| 11:17:02 | sean-k-mooney | they were ligitametly conflciting at the code level too | |
| 11:17:32 | lyarwood | ouch yeah that must have been fun | |
| 11:19:04 | sean-k-mooney | we ended up doing the same ting stacking it all into one big chain | |
| 11:27:51 | openstackgerrit | Balazs Gibizer proposed openstack/python-novaclient master: Microversion 2.86 - Extra spec validation https://review.opendev.org/718057 | |
| 11:27:52 | openstackgerrit | Balazs Gibizer proposed openstack/python-novaclient master: Microversion 2.87 - Stable device boot from volume rescue https://review.opendev.org/714956 | |
| 11:40:21 | openstackgerrit | Merged openstack/nova master: Correct server shelve policy check_str https://review.opendev.org/717542 | |
| 11:46:12 | openstackgerrit | Takashi Natsume proposed openstack/python-novaclient master: Microversion 2.86 - Extra spec validation https://review.opendev.org/718057 | |
| 11:54:49 | openstackgerrit | Takashi Natsume proposed openstack/python-novaclient master: Microversion 2.87 - Stable device boot from volume rescue https://review.opendev.org/714956 | |
| 12:14:55 | openstackgerrit | Alexandre arents proposed openstack/nova master: libvirt: Calculate disk_over_committed for raw instances https://review.opendev.org/717037 | |
| 12:14:57 | bauzas | gibi: back there | |
| 12:15:53 | bauzas | gibi: just upped to +2 https://review.opendev.org/#/c/700812/22 do you want to +W ? | |
| 12:16:04 | gibi | bauzas: no, go ahead with +W | |
| 12:16:12 | gibi | and thanks | |
| 12:38:59 | sean-k-mooney | artom: just noticed you had https://review.opendev.org/#/c/672127/1 for https://bugs.launchpad.net/nova/train/+bug/1836389 | |
| 12:39:00 | openstack | Launchpad bug 1836389 in OpenStack Compute (nova) ussuri "Device role tagging doesn't work for SRIOV PF" [Medium,In progress] - Assigned to Artom Lifshitz (notartom) | |
| 12:39:29 | sean-k-mooney | i just went and triaged the bug but are you actully planning to pick that back up since its been 9 months | |
| 12:40:18 | sean-k-mooney | it would be good to fix it but not sure if you will have time between now and rc1 | |
| 12:51:52 | artom | sean-k-mooney, we had another downstream bug for that downstream, I can try to find the link | |