| *** dmellado1 is now known as dmellado | 00:13 | |
| vanou | good morning ironic | 00:31 |
|---|---|---|
| TheJulia | o/ vanou | 00:32 |
| TheJulia | Just as I'm about to call it a night. :) | 00:32 |
| vanou | TheJulia: Hi o/ US is going into night :) | 00:33 |
| TheJulia | Indeex, and I start my day early to have some overlap with Europe | 00:34 |
| TheJulia | err, Indeed | 00:34 |
| vanou | That is good habit when working in global | 00:35 |
| *** zbitter is now known as zaneb | 00:36 | |
| vanou | Japan timezone is middle of US and Europe. So it's possible to have meeting with Europe and US at morning and night in Japan. | 00:40 |
| TheJulia | That is a definite positive | 00:41 |
| vanou | ;) | 00:42 |
| TheJulia | And now my corgi is attempting to get my attention. Time to call it the day | 00:52 |
| vanou | Cute corgi :) Good night o/ | 00:53 |
| stevebaker[m] | And the SIG video is live! https://www.youtube.com/watch?v=azLRhpSS40w&list=PLKqaoAnDyfgoBFAjUvZGjKXQjogWZBLL_ << arne_wiebalck JayF | 04:06 |
| stevebaker[m] | shorter link https://www.youtube.com/watch?v=azLRhpSS40w | 04:08 |
| rpittau | good morning ironic! o/ | 08:54 |
| opendevreview | Merged openstack/metalsmith master: test_provision: don't assert provision_node call order https://review.opendev.org/c/openstack/metalsmith/+/873035 | 09:09 |
| *** dmellado_ is now known as dmellado | 10:29 | |
| kubajj | good morning everyone! | 11:29 |
| iurygregory | good morning Ironic | 12:15 |
| *** dmellado_ is now known as dmellado | 12:57 | |
| kubajj | dtantsur, TheJulia: if you had a moment, could I ask for a re-review just to finish this change? https://review.opendev.org/c/openstack/ironic/+/871394/ | 13:27 |
| dtantsur | kubajj: sorry, might have missed a few things on the previous pass. a new review incoming. | 13:38 |
| kubajj | dtantsur: no worries, I am happy to fix it | 13:38 |
| kubajj | dtantsur: by this https://review.opendev.org/c/openstack/ironic/+/871394/comments/130c65a6_01431352 , do you mean just trying to return instead of the try except block? | 13:46 |
| dtantsur | kubajj: my first comment was about a redundant temporary variable 'ret'. then I discovered the relationship between the exceptions, which mean that you can probably just let it propagate (i.e. remove try..except). | 13:47 |
| kubajj | dtantsur: Ok, will do, thanks | 13:47 |
| kubajj | dtantsur: For the exceptions, do you see the context (if it's nostore, swift, etc) useful, or should I just use the NodeInventoryNotFound as is? | 13:49 |
| dtantsur | kubajj: I'm not sure if it's of much help for end users | 13:52 |
| kubajj | Ok | 13:52 |
| opendevreview | Jakub Jelinek proposed openstack/ironic master: Erase swift inventory entry on node deletion https://review.opendev.org/c/openstack/ironic/+/871394 | 14:05 |
| kubajj | dtantsur: If there is anything else to change, let me know, but this should resolve all your comments ^ | 14:13 |
| dtantsur | responded | 14:26 |
| kubajj | dtantsur: should I pass just the node.uuid to the exception or rather something like 'for node <node.uuid>' | 14:32 |
| dtantsur | kubajj: you can rephrase the error message for readability, yes | 14:32 |
| dtantsur | (not really "pass", more of changing the class) | 14:32 |
| opendevreview | Jakub Jelinek proposed openstack/ironic master: Erase swift inventory entry on node deletion https://review.opendev.org/c/openstack/ironic/+/871394 | 14:40 |
| kubajj | dtantsur: Oh, I did not see the latter comment. I assume that this is not the way then ^ | 14:41 |
| kubajj | I wanted it to make sense for both the db errors and the nostore/swift exceptions | 14:42 |
| dtantsur | kubajj: if you pass the string this way, it's not translatable (while error messages are - see _(..) around them) | 14:42 |
| dtantsur | I would assume db errors are also for a node? | 14:42 |
| dtantsur | there is no usable inventory ID if I recall it right | 14:43 |
| * TheJulia tries to wake up | 14:43 | |
| kubajj | dtantsur: So I looked into it and it seems that it was node_id that I passed to it. And this seems wrong, right? https://review.opendev.org/c/openstack/ironic/+/862569/11/ironic/db/sqlalchemy/api.py#2578 | 14:46 |
| kubajj | Because I am passing node_id=... but the argument is inventory, right? | 14:46 |
| dtantsur | kubajj: yeah, feels wrong. also, node_id (the numeric one) is not public, so we may need to re-raise it with the node's UUID | 14:49 |
| kubajj | dtantsur: should I change it in this change? (it is a follow-up to an api change, not db) | 14:50 |
| dtantsur | kubajj: probably separately unless we're making it worse :) | 14:51 |
| opendevreview | Jakub Jelinek proposed openstack/ironic master: Erase swift inventory entry on node deletion https://review.opendev.org/c/openstack/ironic/+/871394 | 14:57 |
| kubajj | dtantsur: btw, by _ being translatable, is this for localization? | 14:58 |
| JayF | dtantsur: mind pointing a review at my sharding changes, too? They are complete, only waiting review... | 15:19 |
| JayF | https://review.opendev.org/c/openstack/ironic/+/864236 is the first of 4 in that chain | 15:20 |
| JayF | I got a review from steve on that first one, nice, I didn't see that | 15:22 |
| dtantsur | JayF: do you want to follow-up on Steve's comments first? | 15:36 |
| dtantsur | kubajj: yep | 15:36 |
| JayF | dtantsur: I already commented on the ones I'm following, and the ones I have further questions on | 15:37 |
| JayF | you should be able to see how it looks to you if you want to | 15:37 |
| JayF | I would anticipate it being a little bit of time before I get those changes complete... | 15:37 |
| JayF | also steve is enough core review to land it | 15:38 |
| kubajj | dtantsur: I will create the change fixing the db exception handling once the current one is merged. | 15:38 |
| JayF | so also judge that against your available time | 15:38 |
| rpittau | bye everyone, have a great weekend! o/ | 16:15 |
| JayF | take a look at the recent openstack-discuss list post | 16:19 |
| JayF | sounds like someone wants to implement OAUTH in keystonemiddleware | 16:19 |
| JayF | which would be a huge, huge boon for standalone ironic | 16:19 |
| TheJulia | omg yes | 16:20 |
| dtantsur | mmmm, neat | 16:22 |
| TheJulia | a... oauth group to rbac role translation thing would be needed | 16:46 |
| TheJulia | but aside from that.... it could be epic | 16:46 |
| arne_wiebalck | bye everyone, have a good weekend o/ | 17:18 |
| JayF | TheJulia: looking at the patch, it looks like that might be config | 18:43 |
| JayF | TheJulia: the amount I'm jazzed about this idea is HUGE | 18:43 |
| * JayF would love to see an openstack deployment with no keystone whatsoever | 18:44 | |
| JayF | (not because I dislike keystone; but because anytime we can remove a service it drops the complexity a HUGE amount) | 18:44 |
| TheJulia | Agreed | 18:58 |
| TheJulia | Also, just supporting native oauth capabilities allows one to bolt ironic or other services into an existing environment | 18:59 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!