| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-08-31 | |||
| 13:03:37 | sean-k-mooney | although i belive he is off today | |
| 13:03:59 | sean-k-mooney | let me check his ironic series and see if he has done that convertion | |
| 13:06:09 | sean-k-mooney | Uggla: so for ironic stpehin is using | |
| 13:06:11 | sean-k-mooney | self._ironic_connection = utils.get_sdk_adapter( | |
| 13:06:13 | sean-k-mooney | 'baremetal', check_service=True) | |
| 13:06:47 | sean-k-mooney | which is https://github.com/openstack/nova/blob/master/nova/utils.py#L969-L996 | |
| 13:07:29 | Uggla | yep, I use the same "helper". But I think it does not manage service token. | |
| 13:08:36 | sean-k-mooney | so its calling ks_loading.load_session_from_conf_options | |
| 13:09:51 | Uggla | yes | |
| 13:11:01 | sean-k-mooney | so no that does not look like it load the service user section | |
| 13:11:20 | sean-k-mooney | it ends up calling https://opendev.org/openstack/keystoneauth/src/commit/8d24892f9dc2742d9bd837b3a71fce260a5ec326/keystoneauth1/loading/session.py#L235-L258 | |
| 13:13:18 | Uggla | sean-k-mooney, yep so no service token right ? | |
| 13:13:49 | sean-k-mooney | no so you will need to figure out how to do htat | |
| 13:14:00 | sean-k-mooney | im looking at there docs quickly to see if they have an example | |
| 13:14:08 | sean-k-mooney | i think its part of the session but that just a guess | |
| 13:17:29 | Uggla | sean-k-mooney, no worries at least it confirms my thoughts. | |
| 13:19:59 | sean-k-mooney | i belive there is an #openstack-sdk or #openstack-clinets channel? | |
| 13:22:17 | frickler | actually #openstack-sdks for added confusion | |
| 13:23:27 | sean-k-mooney | frickler: well the sdk does not import service_token form kestoneauth... | |
| 13:23:37 | sean-k-mooney | so it looks liek it does not supprot them at all | |
| 13:23:40 | sean-k-mooney | which is a probelm | |
| 13:24:51 | kashyap | sean-k-mooney: Thanks for this other-kind-of-bump :) : https://review.opendev.org/c/openstack/nova/+/893360 | |
| 13:24:51 | bauzas | ooooh ? | |
| 13:25:12 | kashyap | (And the recheck on the version-bump patch itself) | |
| 13:25:24 | sean-k-mooney | bauzas: https://codesearch.opendev.org/?q=from+keystoneauth1+import+service_token&i=nope&literal=nope&files=&excludeFiles=&repos= | |
| 13:25:30 | frickler | I don't know anything about these details, just wanted to point out the proper channel name. gtema and stephenfin over there can probably be much more helpful than me | |
| 13:25:59 | sean-k-mooney | frickler: no worries knwoign it #openstack-sdks helps :) | |
| 13:26:30 | sean-k-mooney | so this is hwo nova enabels service tokens | |
| 13:26:32 | sean-k-mooney | https://opendev.org/openstack/nova/src/branch/master/nova/service_auth.py#L38-L54 | |
| 13:26:43 | bauzas | sean-k-mooney: doh | |
| 13:28:00 | sean-k-mooney | we might just be able to externally wrap the sdk object | |
| 13:28:11 | sean-k-mooney | but the sdk shoudl be update to supprot this nativly in conenct | |
| 13:30:10 | Uggla | sean-k-mooney, I will ask in the sdks channel. | |
| 13:31:16 | opendevreview | John Garbutt proposed openstack/nova master: Limit nodes by ironic shard key https://review.opendev.org/c/openstack/nova/+/886980 | |
| 13:35:13 | bauzas | dansmith: could you save me a couple of mins ? do you remember top of your head which helper method in o.vo can do a deep copy of a list of objects ? | |
| 13:35:28 | bauzas | copy.deepcopy() can't work correctly | |
| 13:36:09 | bauzas | and obj_clone() is IIRC only for single objects | |
| 13:36:11 | bauzas | but I can try | |
| 13:38:14 | dansmith | bauzas: um, not sure there is anything other than obj_clone | |
| 13:38:24 | bauzas | then that's weird | |
| 13:39:16 | bauzas | https://paste.opendev.org/show/b1nUZLuwjL1jJ0eFsTA5/ | |
| 13:39:16 | dansmith | you could obj_from_primitive(obj_to_primitive()) | |
| 13:39:40 | bauzas | so basically the containers aren't identical | |
| 13:40:00 | dansmith | well, if it made a copy of the objects inside then bdm1 is not in the copy list by definition | |
| 13:40:15 | bauzas | damn, right | |
| 13:40:37 | bauzas | it's a deepcopy, the reference is different | |
| 13:41:33 | dansmith | I'll say again, I really don't think there's anything wrong with modifying the list in place | |
| 13:41:40 | bauzas | auniyal: you know what ? instead of removing items from a copied list, just add items into a new empty list at start | |
| 13:41:42 | auniyal | copy.copy worked | |
| 13:41:43 | dansmith | in fact it seems *more* right to me to do so | |
| 13:41:57 | dansmith | please don't copy.copy() an object | |
| 13:42:06 | auniyal | enpty list - not of BlockDeviceMapping | |
| 13:42:10 | auniyal | is it okay | |
| 13:42:46 | auniyal | this works | |
| 13:42:47 | auniyal | https://paste.opendev.org/show/bRTqAnAkcUd9XQEhLiqP/ | |
| 13:43:30 | dansmith | please don't do that | |
| 13:43:38 | bauzas | auniyal: as dansmith said, this is wrong, you're also deleting the original object | |
| 13:43:47 | auniyal | no it didn't other failed | |
| 13:43:55 | auniyal | ack | |
| 13:46:08 | auniyal | bauzas, dansmith and return the list ? | |
| 13:46:34 | bauzas | so, I'm wrapping my head for a simple pattern | |
| 13:47:06 | bauzas | you have many possibilities : | |
| 13:47:49 | bauzas | 1/ iterate over the list, mark the ones to delete, delete them at once in a comprehension list | |
| 13:48:25 | bauzas | 2/ generate an empty object list, iterate over the list, add every single bdm that's correct | |
| 13:48:40 | bauzas | I'm in favor of 1/ | |
| 13:48:46 | dansmith | me too | |
| 13:49:12 | bauzas | 3/ you can use python's filter() with a lambda method | |
| 13:49:16 | dansmith | and I'll say, you don't even need to do the comprehension list part, | |
| 13:49:17 | bauzas | :) | |
| 13:49:26 | dansmith | just make the list of ones to delete, then go back and delete them | |
| 13:49:41 | bauzas | that too | |
| 13:49:53 | dansmith | which I believe is what he already had before we startedin on this | |
| 13:51:30 | auniyal | 1 - iterate over the list; delete(bdm) if culprit else empty_list.append(bdm); return empty_list ?? | |
| 13:52:03 | auniyal | dansmith, so delete part is not happening, | |
| 13:52:54 | dansmith | no, don't return an empty list of course | |
| 13:52:54 | auniyal | thats I am doing here - https://review.opendev.org/c/openstack/nova/+/882284/53/nova/compute/manager.py | |
| 13:53:31 | dansmith | auniyal: just return nothing, as was said there would be fine | |
| 13:53:56 | dansmith | to "make it clear that we're modifying in place". I don't think it's important to do that but if that matters to someone else, then fine | |
| 13:54:39 | dansmith | "for now i would prefer if you just drop the return value and we can adress this later." | |
| 13:54:42 | dansmith | that's all you have to do | |
| 13:54:53 | auniyal | ack, thanks dansmith | |
| 13:55:00 | bauzas | in other words, step back to PS5x-something | |
| 13:55:09 | dansmith | *facepalm* yeah | |
| 13:55:21 | bauzas | actually, not | |
| 13:55:45 | bauzas | I stupidely gave you the wrong IndexError pattern which is to drop objects in a loop | |
| 13:56:00 | dansmith | just roll from here, delete the return value and it's good right? | |
| 13:56:14 | bauzas | I'm lost between versions tbh, but yeah | |
| 14:00:52 | sean-k-mooney | delete the return and let it update in palce as it was and its fine | |
| 14:02:33 | sean-k-mooney | its not quite the same as the old patchset as that was missing assert in the unit tests i think | |
| 14:02:39 | auniyal | yeah it passing all required tests | |
| 14:02:49 | auniyal | yes, I updated tests | |
| 14:03:40 | opendevreview | Amit Uniyal proposed openstack/nova master: Delete dangling bdms https://review.opendev.org/c/openstack/nova/+/882284 | |
| 14:05:28 | sean-k-mooney | auniyal: you forgot the doc string | |
| 14:05:35 | sean-k-mooney | returns: BlockDeviceMappingList list object. | |
| 14:05:52 | auniyal | oh | |
| 14:07:18 | auniyal | sean-k-mooney, can you please here as well https://review.opendev.org/c/openstack/nova/+/882284/53..54/doc/source/admin/manage-volumes.rst | |
| 14:07:24 | auniyal | if its fixed as per comments | |
| 14:07:45 | sean-k-mooney | yep i already reviewd it and marked it doen | |
| 14:07:55 | auniyal | ack, thanks | |
| 14:08:05 | sean-k-mooney | the doc string is all that left so if you fix that ill add a +2 | |