| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-03-10 | |||
| 13:32:15 | sean-k-mooney | its a clase we use as part of the prtty prining of data | |
| 13:32:29 | sean-k-mooney | its part of how dicts are rendered using click/cliff | |
| 13:32:54 | zigo | All of this, I don't really mind much, but IMO, it's going to confuse users a lot ! | |
| 13:33:08 | sean-k-mooney | zigo: it kind of looks like someone wen through all the files that could be returned and rendered them by default | |
| 13:33:24 | sean-k-mooney | zigo: right but hte nova team is not really invovled in osc | |
| 13:33:26 | zigo | Also, why do we now have attached_volumes AND volumes_attached ? Do we need this *TWICE* ? :) | |
| 13:33:45 | sean-k-mooney | like we were not asked about any of these changes | |
| 13:34:41 | zigo | Ok. | |
| 13:35:19 | sean-k-mooney | https://github.com/openstack/python-openstackclient/commit/794334ec2405bcfe086b3a56c796a9b6c2f7c685 might be related | |
| 13:35:44 | sean-k-mooney | it may have incorectly resulting in --long effectilgy being always used | |
| 13:36:27 | sean-k-mooney | no its this https://github.com/openstack/python-openstackclient/commit/70dbb01ea3ed900a41092d46ed5ae1370d5771af | |
| 13:36:53 | sean-k-mooney | they swap to the sdk but they added a bunch of fields at the same time | |
| 13:39:53 | sean-k-mooney | i would proably have gated the sdk names behind a flag or somethign and doen it in two patchs | |
| 13:40:03 | sean-k-mooney | first just to swap to sdk with current behavior | |
| 13:40:42 | sean-k-mooney | also the client shoudl really use the names of the field n the api resopnce if possible | |
| 13:41:03 | sean-k-mooney | so im not sure we should use the sdk names at all | |
| 13:41:33 | sean-k-mooney | the ones that novaclient used are the ones form the api responce so i proably would have -1 that patch if we had been asked to review | |
| 13:41:49 | sean-k-mooney | stephenfin: ^ for awareness | |
| 13:42:55 | sean-k-mooney | give there have now been two release with that i assume its two late to revert it and disucss this with the wider nova team? | |
| 13:44:18 | sean-k-mooney | we should at least add it to the ptg adgenda i think to disucss this or have a mailing list thread on the topic | |
| 13:45:55 | sean-k-mooney | i dont nessicarly disagree that the new names are more consitent but it breaks the idea that if you want to include a coluem you use -c with the api field | |
| 13:55:31 | zigo | I agree with all you wrote above. :) | |
| 13:55:43 | sean-k-mooney | https://review.opendev.org/c/openstack/python-openstackclient/+/877017 | |
| 13:55:53 | sean-k-mooney | i propsoed a revert so we can discuss the way forward | |
| 13:56:01 | sean-k-mooney | ill add it to the ptg adgenda | |
| 13:56:41 | zigo | You may want to review your patch header (typoes...) :) | |
| 14:21:51 | gibi | bauzas: how do you feel about requiring that share_mapping.id is an sa.BigInteger from the start? | |
| 14:23:28 | bauzas | gibi: good question, I have a PTG topic about it | |
| 14:23:42 | bauzas | maybe not all the tables, but I dunno for this one | |
| 14:24:12 | gibi | bauzas: I will leave a comment to Uggla's but if there is no consensus yet on this then I will keep this optional | |
| 14:24:27 | Uggla | \o/ | |
| 14:24:34 | bauzas | gibi: afaik, all our FK ids are not BigInteger yet | |
| 14:25:41 | gibi | yeah, I just thought that if we already know that we want to increase the key space from sa.Integer to a bigger one to avoid the overflow we saw, then maybe want to have share_mapping with a big key from the start so that table does not need to be changed later | |
| 14:26:05 | bauzas | yup meh to me | |
| 14:32:24 | sean-k-mooney | if we are adding new id fiels i would prefer to use unsigned big ints | |
| 14:32:59 | sean-k-mooney | basically uint64_t in c++ terms | |
| 14:42:25 | bauzas | sean-k-mooney: SQLA doesn't support unsigned ints by default AFAIK https://docs.sqlalchemy.org/en/20/core/types.html | |
| 14:42:42 | bauzas | so you would need to setup an unsigned int object with a mysql variant | |
| 14:43:10 | bauzas | anyway, let's not open this can of worms on a Friday afternoon | |
| 14:44:28 | sean-k-mooney | you have ot use the dialect form yes i know | |
| 14:44:57 | sean-k-mooney | but but what if i want to ruine you weekend :P | |
| 14:45:40 | sean-k-mooney | we can talk about it next week or on the review but i would at least use a BigInteger field | |
| 14:45:51 | sean-k-mooney | 63 bits is still better then 31 | |
| 14:46:05 | bauzas | well, my weenkend is somehow already ruined, the snow isn't there here and the wind is acting like a hairdryer on the very few left snow | |
| 14:47:03 | sean-k-mooney | if we dont want to rely on https://docs.sqlalchemy.org/en/20/dialects/mysql.html#sqlalchemy.dialects.mysql.BIGINT.params.unsigned then im fine with just the generic bigint | |
| 14:47:12 | sean-k-mooney | this is going to be a ptg topic anyway | |
| 14:47:34 | sean-k-mooney | bauzas: we had snow here last night | |
| 14:47:44 | sean-k-mooney | your welcome to come take it back :P | |
| 14:56:04 | bauzas | sean-k-mooney: that depends, how many tracks do you have here ? | |
| 14:56:14 | bauzas | :p | |
| 14:57:08 | bauzas | https://webcams.lecollet.com/imageSC.jpg when I see this, I cry | |
| 15:00:07 | bauzas | dansmith: if you have a bit of time, could we discuss about https://review.opendev.org/c/openstack/nova/+/875621/2 ? | |
| 15:00:26 | bauzas | dansmith: I'm not really a grenade expert, so I need to make sure I understood it correctly | |
| 15:01:14 | dansmith | bauzas: sure, gmann wants to wait to merge that until devstack and grenade get branched | |
| 15:01:21 | dansmith | which usually happens a bit after the last regular project | |
| 15:01:30 | bauzas | ack ok | |
| 15:01:41 | dansmith | (the dependent patch I mean) | |
| 15:01:46 | bauzas | I'll look at the open grenade changes then | |
| 15:02:16 | bauzas | dansmith: actually, I haven't done https://review.opendev.org/c/openstack/nova/+/875621/2 depending on your skip-level-always zuul patch | |
| 15:02:33 | dansmith | oh, it's merged now, I missed that | |
| 15:02:59 | bauzas | but, it still fails for the same reason : grenade has an original branch which is zed and tries to update to master, which is now Bobcat, hence the grenade job failing | |
| 15:03:24 | bauzas | so we need to update grenade to have an original branch to be antelope | |
| 15:03:45 | dansmith | you mean for regular grenade jobs? | |
| 15:03:57 | bauzas | yep, see my patch : https://review.opendev.org/c/openstack/nova/+/875621/2 | |
| 15:03:58 | dansmith | the skip-always job should be zed->bobcat | |
| 15:04:18 | bauzas | it fails on grenade-multinode https://zuul.opendev.org/t/openstack/build/cd27f6d6a4094fd48b1725e1f63098f1 | |
| 15:04:32 | bauzas | (which is an expected behaviour, if we upgrade from zed to master) | |
| 15:05:06 | dansmith | the grenade job as defined in grenade's zuul config should be moving to antelope | |
| 15:05:26 | bauzas | ok, that's what I understood and wrote in my last comment then | |
| 15:05:34 | bauzas | https://review.opendev.org/c/openstack/nova/+/875621/2#message-cef3c3bb3d8a590cd4d9bf51267a7fe092bd32b2 | |
| 15:05:42 | dansmith | that happens after all the projects are branched | |
| 15:06:03 | bauzas | okay | |
| 15:06:11 | bauzas | that's what I guessed | |
| 15:06:20 | bauzas | the service projects I guess, not the trailing ones | |
| 15:06:31 | dansmith | you're moving your OLDEST_SUPPORTED, but will use the workaround to prevent that from breaking on the skip-always job? | |
| 15:06:45 | dansmith | bauzas: anything that runs a grenade job I think | |
| 15:06:47 | bauzas | in my patch ? | |
| 15:06:52 | dansmith | yes | |
| 15:07:14 | bauzas | well, in my patch, I'm just saying we continue to only support N-1 nodes for a non-SLURP release | |
| 15:07:29 | bauzas | even if skip-always job workarounds it | |
| 15:07:41 | bauzas | hence the service version be Antelope | |
| 15:08:10 | bauzas | once grenade is modified to have a source env to be Antelope, this will work | |
| 15:08:18 | dansmith | that's what I just said yeah | |
| 15:08:27 | bauzas | and this is now unrelated to the skip-always job you're doing | |
| 15:08:41 | bauzas | since the skip-always is using the workaround flag | |
| 15:08:46 | dansmith | right | |
| 15:08:51 | bauzas | coo col | |
| 15:09:11 | bauzas | dansmith: my ping was just for making sure I was understand the job failure correctly | |
| 15:09:18 | bauzas | understanding* | |
| 15:09:32 | bauzas | thanks so | |
| 15:09:36 | bauzas | I'll wait | |
| 15:10:38 | dansmith | yeah, I dunno what gmann's schedule is for that.. we could put up the grenade change for you to depends-on I think | |
| 15:11:02 | dansmith | but i mean, you're bumping the min version, and it's failing on the min version check, because it's still upgrading from zed, so ... pretty straightforward | |
| 15:11:05 | bauzas | yup, I'll track the open grenade changes | |
| 15:11:45 | bauzas | having a depends-on is nice, because next cycle, people who would write this service version bump would remember we need to hold until grenade is updated | |
| 15:11:56 | bauzas | call it a documentation :) | |
| 15:12:02 | bauzas | actually | |
| 15:12:14 | bauzas | we won't bump next cycle, as this will be SLURP | |
| 15:12:32 | dansmith | I mean, it's pretty clear why it's failing no? :) | |
| 15:12:33 | bauzas | so the next patch to update the min will be beginning of D, in one year | |