| * jroll heads home | 00:00 | |
| jroll | have a good night everybody :) | 00:01 |
|---|---|---|
| NobodyCam | night jroll :) | 00:01 |
| *** yuanying has quit IRC | 00:01 | |
| rloo | bye jroll | 00:02 |
| rloo | mrda: not sure I understand. 111223 has 3 +2's, why do you want us to +1 it? | 00:03 |
| mrda | rloo: well, not that one, from 111425 onwards | 00:04 |
| mrda | there's no +2s there, so I just want to bring it to Nova's attention | 00:05 |
| rloo | mrda: ok | 00:05 |
| rloo | mrda: qq, I think I might be too tired to review. https://review.openstack.org/#/c/111425/19/nova/virt/ironic/driver.py | 00:11 |
| rloo | mrda: why is line 43 not py_logging.getLogger(...)? | 00:12 |
| rloo | mrda: yes, I'm tired. the answer is cuz logging is from something else, not from logging. I'll look at it tomorrow. | 00:14 |
| mrda | rloo: if you're too tired, take an early mark :) | 00:14 |
| NobodyCam | 115540 has -1's :( | 00:14 |
| mrda | rloo: always appreciate your help | 00:15 |
| rloo | mrda: I didn't (or forgot) realize that we should be reviewing these too. They weren't on my radar. Sorry. | 00:15 |
| NobodyCam | oh wow are thouse from rev 3/4? | 00:16 |
| mrda | NobodyCam: that's only because Daniel B and Dan S haven't re-reviewed since the proposed solution to their objections was posted | 00:16 |
| mrda | NobodyCam: yep | 00:16 |
| mrda | thanks Labor Day :( | 00:16 |
| NobodyCam | hehehe | 00:17 |
| *** dlaube has joined #openstack-ironic | 00:17 | |
| openstackgerrit | Kyle Stevenson proposed a change to openstack/ironic: Reduce redundancy in conductor manager docstrings https://review.openstack.org/118505 | 00:17 |
| mrda | rloo: I hadn't asked, but we're now getting to the point where we keep them in front of nova cores if at all possible | 00:17 |
| rloo | mrda: does nova have something set so that reviewers' scores get carried forward to new revisions? | 00:18 |
| mrda | rloo: yeah, I think this is as a result of the last gerrit update (where the CI toggle came in) | 00:18 |
| mrda | It's new to me | 00:18 |
| * mrda is bugging nova cores privately too | 00:18 | |
| rloo | mrda. that doesn't happen in Ironic. interesting. ok, for sure, will look at the nova driver patches tomorrow. | 00:19 |
| mrda | rloo: thanks, much appreciated! | 00:19 |
| *** chuckC has quit IRC | 00:20 | |
| *** dlaube has quit IRC | 00:20 | |
| *** penick has quit IRC | 00:30 | |
| *** penick has joined #openstack-ironic | 00:37 | |
| JoshNang | horizon api wrapper should be done. i'd love some eyeballs on it to make sure its not completely terrible. https://review.openstack.org/#/c/117376/ | 00:40 |
| *** penick has quit IRC | 00:51 | |
| *** penick has joined #openstack-ironic | 00:57 | |
| *** yuanying has joined #openstack-ironic | 00:59 | |
| *** r-daneel has quit IRC | 01:00 | |
| *** lnxnut has joined #openstack-ironic | 01:06 | |
| *** yuanying has quit IRC | 01:07 | |
| *** lnxnut has quit IRC | 01:16 | |
| *** eguz has joined #openstack-ironic | 01:26 | |
| *** eghobo has quit IRC | 01:26 | |
| *** rushiagr_away is now known as rushiagr | 01:29 | |
| openstackgerrit | A change was merged to openstack/ironic: Fix typo in PXE driver docstrings https://review.openstack.org/118495 | 01:36 |
| *** bandicot has quit IRC | 01:41 | |
| *** rushiagr is now known as rushiagr_away | 01:45 | |
| *** penick has quit IRC | 01:46 | |
| openstackgerrit | Haomeng,Wang proposed a change to openstack/ironic: Remove ironic.conf.sample https://review.openstack.org/106493 | 01:48 |
| *** penick has joined #openstack-ironic | 01:49 | |
| *** penick has quit IRC | 01:49 | |
| *** pcrews has quit IRC | 01:50 | |
| *** eguz has quit IRC | 01:50 | |
| *** pcrews has joined #openstack-ironic | 01:52 | |
| *** nosnos has joined #openstack-ironic | 01:52 | |
| mrda | hey, NobodyCam? | 01:52 |
| mrda | You still here? | 01:52 |
| NobodyCam | no | 01:59 |
| NobodyCam | :-p | 01:59 |
| NobodyCam | mrda: whatsup? | 01:59 |
| *** yongli has quit IRC | 02:02 | |
| *** yongli has joined #openstack-ironic | 02:03 | |
| *** yuanying has joined #openstack-ironic | 02:04 | |
| mrda | NobodyCam: thanks for responding. It's ok, I'll leave it for now. Have a good night! | 02:07 |
| *** rushiagr_away is now known as rushiagr | 02:08 | |
| NobodyCam | lol | 02:12 |
| NobodyCam | ok :) | 02:12 |
| jroll | mrda: I'm around for a bit if you need something | 02:12 |
| NobodyCam | I am here | 02:12 |
| *** yuanying has quit IRC | 02:12 | |
| NobodyCam | I was kidding about the no | 02:12 |
| jroll | I have to be here :P | 02:12 |
| mrda | sure, I was getting some private questions over irc on the ironic driver, but I need to qualify what the question actually _is_ before asking you guys :) | 02:13 |
| NobodyCam | lol I have game open in other window | 02:13 |
| jroll | heh, ok | 02:13 |
| jroll | make them talk in channel :P | 02:13 |
| mrda | :) | 02:13 |
| NobodyCam | :) | 02:13 |
| jroll | I'm mostly serious | 02:14 |
| jroll | that's really lame | 02:14 |
| mrda | jroll: it's all good | 02:15 |
| jroll | ok, if you say so | 02:15 |
| *** nosnos has quit IRC | 02:16 | |
| *** chuckC has joined #openstack-ironic | 02:19 | |
| *** rloo has quit IRC | 02:25 | |
| *** pcrews has quit IRC | 02:28 | |
| *** harlowja is now known as harlowja_away | 02:52 | |
| *** rushiagr is now known as rushiagr_away | 03:00 | |
| *** yuanying has joined #openstack-ironic | 03:09 | |
| *** yuanying has quit IRC | 03:18 | |
| *** zz_naotok is now known as naotok | 03:22 | |
| *** eghobo has joined #openstack-ironic | 03:32 | |
| *** Poornima has joined #openstack-ironic | 03:42 | |
| *** eghobo has quit IRC | 03:49 | |
| openstackgerrit | Haomeng,Wang proposed a change to openstack/ironic: Add send-data-to-ceilometer support for pxe_ipminative driver https://review.openstack.org/112486 | 04:06 |
| *** rushiagr_away is now known as rushiagr | 04:07 | |
| *** eghobo has joined #openstack-ironic | 04:08 | |
| *** yuanying has joined #openstack-ironic | 04:14 | |
| *** jogo has joined #openstack-ironic | 04:16 | |
| jogo | so I am reviewing the nova patches | 04:17 |
| jogo | and have a few questions if anyone is around | 04:17 |
| *** yuanying has quit IRC | 04:22 | |
| *** killer_prince has quit IRC | 04:24 | |
| *** killer_prince has joined #openstack-ironic | 04:38 | |
| *** killer_prince is now known as lazy_prince | 04:39 | |
| *** Nisha has joined #openstack-ironic | 04:40 | |
| *** wanyen has quit IRC | 04:48 | |
| *** nosnos has joined #openstack-ironic | 04:50 | |
| *** jogo has left #openstack-ironic | 04:53 | |
| *** rameshg87 has joined #openstack-ironic | 04:59 | |
| *** rakesh_hs has joined #openstack-ironic | 05:02 | |
| *** yuanying has joined #openstack-ironic | 05:19 | |
| *** enikanorov__ has quit IRC | 05:23 | |
| openstackgerrit | Syed Ismail Faizan Barmawer proposed a change to openstack/ironic: Add uefi boot mode support in IloVirtualMediaIscsiDeploy https://review.openstack.org/116561 | 05:26 |
| *** yuanying has quit IRC | 05:27 | |
| openstackgerrit | Ramakrishnan G proposed a change to openstack/ironic: IloVirtualMediaIscsi deploy driver https://review.openstack.org/113865 | 05:28 |
| openstackgerrit | Ramakrishnan G proposed a change to openstack/ironic: IloVirtualMediaIscsi deploy driver https://review.openstack.org/113865 | 05:33 |
| *** bmahalakshmi has joined #openstack-ironic | 05:45 | |
| openstackgerrit | OpenStack Proposal Bot proposed a change to openstack/ironic: Imported Translations from Transifex https://review.openstack.org/118548 | 06:09 |
| *** k4n0 has joined #openstack-ironic | 06:18 | |
| *** eghobo has quit IRC | 06:21 | |
| *** yuanying has joined #openstack-ironic | 06:24 | |
| *** ndipanov_gone has quit IRC | 06:26 | |
| *** eghobo has joined #openstack-ironic | 06:27 | |
| *** lazy_prince is now known as killer_prince | 06:28 | |
| *** ndipanov has joined #openstack-ironic | 06:29 | |
| *** killer_prince has quit IRC | 06:31 | |
| openstackgerrit | Ramakrishnan G proposed a change to openstack/ironic: IloVirtualMediaAgent deploy driver https://review.openstack.org/115885 | 06:31 |
| *** lazy_prince has joined #openstack-ironic | 06:31 | |
| *** yuanying has quit IRC | 06:32 | |
| openstackgerrit | Ramakrishnan G proposed a change to openstack/ironic: IloVirtualMediaAgent deploy driver https://review.openstack.org/115885 | 06:37 |
| *** rameshg87 has quit IRC | 06:40 | |
| *** rameshg87 has joined #openstack-ironic | 06:40 | |
| *** jcoufal has joined #openstack-ironic | 06:43 | |
| *** eghobo has quit IRC | 06:45 | |
| *** lazy_prince has quit IRC | 06:53 | |
| *** lazy_prince has joined #openstack-ironic | 06:53 | |
| *** foexle has joined #openstack-ironic | 06:56 | |
| *** lazy_prince is now known as killer_prince | 07:03 | |
| *** r-mibu has quit IRC | 07:03 | |
| *** bluex-pl has joined #openstack-ironic | 07:04 | |
| *** lsmola has quit IRC | 07:16 | |
| *** yuanying has joined #openstack-ironic | 07:29 | |
| *** wendar_ is now known as wendar | 07:36 | |
| *** yuanying has quit IRC | 07:38 | |
| *** jistr has joined #openstack-ironic | 07:53 | |
| *** killer_prince is now known as lazy_prince | 08:04 | |
| *** lsmola has joined #openstack-ironic | 08:07 | |
| *** MattMan has quit IRC | 08:09 | |
| *** MattMan has joined #openstack-ironic | 08:10 | |
| *** vdrok_afk is now known as vdrok | 08:10 | |
| *** romcheg1 has joined #openstack-ironic | 08:12 | |
| openstackgerrit | Ramakrishnan G proposed a change to openstack/ironic: IloVirtualMediaIscsi deploy driver https://review.openstack.org/113865 | 08:14 |
| *** athomas has joined #openstack-ironic | 08:17 | |
| *** bluex-pl has quit IRC | 08:20 | |
| *** bluex-pl has joined #openstack-ironic | 08:25 | |
| *** derekh has joined #openstack-ironic | 08:25 | |
| *** bluex-pl has quit IRC | 08:25 | |
| *** lucasagomes has joined #openstack-ironic | 08:30 | |
| *** yuanying has joined #openstack-ironic | 08:34 | |
| *** yuanying has quit IRC | 08:43 | |
| *** bluex-pl has joined #openstack-ironic | 08:48 | |
| *** stelfer has joined #openstack-ironic | 08:52 | |
| *** jistr has quit IRC | 09:06 | |
| openstackgerrit | Ramakrishnan G proposed a change to openstack/ironic: IloVirtualMediaIscsi deploy driver https://review.openstack.org/113865 | 09:07 |
| *** dtantsur|afk is now known as dtantsur | 09:07 | |
| dtantsur | Morning Ironic | 09:07 |
| romcheg1 | Good morning dtantsur! | 09:08 |
| dtantsur | romcheg1, morning | 09:09 |
| *** jistr has joined #openstack-ironic | 09:10 | |
| openstackgerrit | Ramakrishnan G proposed a change to openstack/ironic: Support for setting boot mode in pxe_ilo driver https://review.openstack.org/118578 | 09:28 |
| *** rameshg87 has quit IRC | 09:37 | |
| *** yuanying has joined #openstack-ironic | 09:39 | |
| *** rameshg87 has joined #openstack-ironic | 09:44 | |
| *** rameshg87 has left #openstack-ironic | 09:46 | |
| stelfer | Hi all - I have a question about ManagementInterface. Our power driver can only control power outlets. Boot device is not something we can control. In our system, we use PXE for controlling deployment/boot parameters. Isn't there a default ManagementInterface instance we can use when we have no management capability? | 09:48 |
| *** yuanying has quit IRC | 09:48 | |
| *** yuriyz has quit IRC | 09:53 | |
| dtantsur | stelfer, hi! seems like the only option is having management=None | 09:54 |
| romcheg1 | dtantsur: will it work? | 09:54 |
| dtantsur | lemme check | 09:54 |
| stelfer | Are there side effects to that? | 09:54 |
| romcheg1 | dtantsur: I'm going to check the manager | 09:55 |
| dtantsur | it will do nothing if management is None, I checked now | 09:56 |
| dtantsur | stelfer, if boot device for a machine is not PXE, deploy will fail | 09:56 |
| stelfer | That sounds absolutely fine (at least for our config). With our driver, nothing else is available. | 09:57 |
| romcheg1 | yup, otherwise it should work | 09:57 |
| stelfer | Thank you | 09:57 |
| *** viktors|afk is now known as viktors | 09:57 | |
| lucasagomes | yup as dtantsur said when setting the boot device we check if the management is present, and if not it does nothing (mgmt is not a core interface so it can be None) | 09:58 |
| openstackgerrit | Vladyslav Drok proposed a change to openstack/ironic: Translator functions cleanup part 1 https://review.openstack.org/116303 | 10:04 |
| openstackgerrit | Vladyslav Drok proposed a change to openstack/ironic: Translator functions cleanup part 2 https://review.openstack.org/118587 | 10:06 |
| romcheg1 | lucasagomes: Is it defined in our dev-doc? | 10:08 |
| openstackgerrit | Vladyslav Drok proposed a change to openstack/ironic: Translator functions cleanup part 3 https://review.openstack.org/118588 | 10:08 |
| romcheg1 | lucasagomes: it === which interfaces are core ones and which are not | 10:09 |
| lucasagomes | romcheg1, maybe not so explicitly http://docs.openstack.org/developer/ironic/dev/architecture.html#drivers | 10:09 |
| lucasagomes | would be good to say which one are cores and which one are not | 10:09 |
| romcheg1 | lucasagomes: yes, I think that may be clarified better :) | 10:10 |
| *** romcheg1 has quit IRC | 10:11 | |
| openstackgerrit | Vladyslav Drok proposed a change to openstack/ironic: Translator functions cleanup part 4 https://review.openstack.org/118591 | 10:11 |
| lucasagomes | romcheg, +1 | 10:12 |
| dtantsur | so translator much cleanup very patches :D | 10:12 |
| lucasagomes | ouch | 10:12 |
| lucasagomes | we better wait til we get the stuff for J3 in... to avoid merge errors etc | 10:13 |
| dtantsur | yeah, clean up does not need FFE I guess | 10:13 |
| *** mgoddard has joined #openstack-ironic | 10:15 | |
| dtantsur | lucasagomes, I was thinking of approving https://review.openstack.org/#/c/107629/8/ironic/tests/base.py but should not we reraise DbMigrationError in case of SQLite here? | 10:28 |
| lucasagomes | dtantsur, hmmmm I think it's a good point to have an else: raise there | 10:29 |
| lucasagomes | it makes sense to me | 10:29 |
| dtantsur | lucasagomes, mind having a look at https://review.openstack.org/#/c/115974/ ? I'd like it to get into J3 so that we can gather more feedback | 10:34 |
| lucasagomes | dtantsur, will do... just finishing something quick here about the drac stuff | 10:34 |
| vdrok | afternoon Ironic! | 10:41 |
| *** naotok is now known as zz_naotok | 10:41 | |
| dtantsur | vdrok, hi | 10:43 |
| vdrok | hi dtantsur | 10:43 |
| vdrok | dtantsur, lucasagomes, this cleanup stuff gets bigger and bigger :) | 10:44 |
| dtantsur | yeah... | 10:44 |
| *** yuanying has joined #openstack-ironic | 10:44 | |
| *** Nisha has quit IRC | 10:47 | |
| lucasagomes | vdrok, yup heh, morning | 10:47 |
| dtantsur | viktors, around? may I reupload https://review.openstack.org/#/c/107629/ ? | 10:51 |
| *** yuanying has quit IRC | 10:53 | |
| viktors | dtantsur: hi! Sorry, can we discuss it a bit later? | 10:53 |
| viktors | I'm a bit busy at the moment | 10:53 |
| *** Poornima has quit IRC | 10:54 | |
| dtantsur | viktors, I just wanted to land it before J3. my only point is to reraise exception | 10:54 |
| dtantsur | but yeah, if you're busy, let us postpone | 10:54 |
| vdrok | dtantsur, so, do you suggest changing % to , only in places that are modified during cleanup, or should I change all of them? | 10:54 |
| viktors | dtantsur: I'll ping you in a 30-60 minutes, ok? | 10:54 |
| dtantsur | vdrok, my suggestion is to change only what you already change | 10:54 |
| dtantsur | viktors, ack | 10:55 |
| vdrok | dtantsur, ah, ok, will do | 10:55 |
| *** Poornima has joined #openstack-ironic | 10:57 | |
| *** bmahalakshmi has quit IRC | 10:58 | |
| *** marios has quit IRC | 10:59 | |
| *** bmahalakshmi has joined #openstack-ironic | 11:00 | |
| *** marios has joined #openstack-ironic | 11:01 | |
| *** marios has joined #openstack-ironic | 11:02 | |
| *** comstud has quit IRC | 11:03 | |
| *** comstud has joined #openstack-ironic | 11:08 | |
| *** rakesh_hs has quit IRC | 11:08 | |
| openstackgerrit | Lucas Alvares Gomes proposed a change to openstack/ironic: Fix minor issues in the DRAC driver https://review.openstack.org/118397 | 11:10 |
| openstackgerrit | A change was merged to openstack/ironic: Add send-data-to-ceilometer support for pxe_ipminative driver https://review.openstack.org/112486 | 11:11 |
| *** Poornima has quit IRC | 11:14 | |
| *** yuriyz has joined #openstack-ironic | 11:14 | |
| openstackgerrit | A change was merged to openstack/ironic: Reduce redundancy in conductor manager docstrings https://review.openstack.org/118505 | 11:14 |
| *** Poornima has joined #openstack-ironic | 11:23 | |
| viktors | dtantsur: still around? | 11:34 |
| dtantsur | viktors, yep | 11:35 |
| viktors | so as for your comment | 11:35 |
| viktors | do you really think, that `raise` is required? | 11:35 |
| viktors | dtantsur: look also at lines 72-75 with the similar case | 11:36 |
| dtantsur | viktors, yes, otherwise devs will have hard time figuring out why their tests do not work | 11:36 |
| *** pelix has joined #openstack-ironic | 11:36 | |
| * viktors want to drop this class at all | 11:37 | |
| viktors | dtantsur: so what about this logic - https://github.com/openstack/ironic/blob/master/ironic/tests/base.py#L76-L79 ? | 11:38 |
| dtantsur | viktors, what do you mean? | 11:39 |
| viktors | dtantsur: i mean, than in that case we just check, is db-file exist, if yes - we suppose, that DB was initialized correctly and don't check nothing in this case | 11:41 |
| dtantsur | it also sucks | 11:42 |
| dtantsur | lemme think | 11:42 |
| *** lucasagomes is now known as lucas-hungry | 11:42 | |
| dtantsur | viktors, do you mean that we'll never actually hit this exception? | 11:43 |
| dtantsur | because sqlite on file will never get upgraded? | 11:44 |
| viktors | dtantsur: yes, we are not trying to update sqlite file at all | 11:46 |
| *** bluex-pl has quit IRC | 11:48 | |
| dtantsur | viktors, I see. Your NOTE and if check are misleading then. could you drop the if and leave a comment, that this exception won't be risen for sqlite? | 11:48 |
| *** yuanying has joined #openstack-ironic | 11:49 | |
| openstackgerrit | Syed Ismail Faizan Barmawer proposed a change to openstack/ironic: Add UEFI based deployment support in Ironic https://review.openstack.org/114357 | 11:51 |
| viktors | dtantsur: I see no big sense in it, bit I'm ok with this. Also its would be nice to know also lucas-hungry opinion about it, because yesterday we decided to keep old logic in Database.__init__() | 11:51 |
| *** bmahalakshmi has quit IRC | 11:51 | |
| *** bluex-pl has joined #openstack-ironic | 11:52 | |
| bluex-pl | jroll: hi, I would be glad if you could take a look at https://bugs.launchpad.net/ironic/+bug/1314148/comments/2 | 11:53 |
| dtantsur | viktors, I mean the comment says "If we use SQLite with the old DB schema, we" which is misleading, because this case cannot happen IMO | 11:53 |
| dtantsur | you won't reach this line | 11:53 |
| *** yuanying has quit IRC | 11:58 | |
| *** rameshg87 has joined #openstack-ironic | 11:59 | |
| * viktors thinking | 11:59 | |
| *** lsmola has quit IRC | 12:01 | |
| openstackgerrit | Vinay B S proposed a change to openstack/ironic: Adds instructions for deploying instances on real hardware https://review.openstack.org/118614 | 12:01 |
| *** HenryG is now known as HenryG_afk | 12:04 | |
| openstackgerrit | Vinay B S proposed a change to openstack/ironic: Adds instructions for deploying instances on real hardware https://review.openstack.org/118614 | 12:04 |
| *** lsmola has joined #openstack-ironic | 12:10 | |
| *** romcheg1 has joined #openstack-ironic | 12:14 | |
| *** nosnos has quit IRC | 12:16 | |
| *** nosnos has joined #openstack-ironic | 12:17 | |
| *** nosnos has quit IRC | 12:21 | |
| *** rameshg87 has left #openstack-ironic | 12:23 | |
| viktors | lucas-hungry, dtantsur - folks, what do you think,m if I'll rollback changes in file ironic/tests/base.py at all? | 12:24 |
| viktors | in patch https://review.openstack.org/#/c/107629 | 12:25 |
| *** bluex-pl has quit IRC | 12:37 | |
| openstackgerrit | Vladyslav Drok proposed a change to openstack/ironic: Translator functions cleanup part 1 https://review.openstack.org/116303 | 12:40 |
| openstackgerrit | Vladyslav Drok proposed a change to openstack/ironic: Translator functions cleanup part 2 https://review.openstack.org/118587 | 12:41 |
| dtantsur | viktors, it may make sense to split into 2 patches and discuss separately | 12:41 |
| openstackgerrit | Vladyslav Drok proposed a change to openstack/ironic: Translator functions cleanup part 3 https://review.openstack.org/118588 | 12:42 |
| openstackgerrit | Vladyslav Drok proposed a change to openstack/ironic: Translator functions cleanup part 4 https://review.openstack.org/118591 | 12:42 |
| *** rameshg87 has joined #openstack-ironic | 12:43 | |
| *** vdrok is now known as vdrok_afk | 12:43 | |
| *** rloo has joined #openstack-ironic | 12:47 | |
| *** rloo has quit IRC | 12:47 | |
| openstackgerrit | Victor Sergeyev proposed a change to openstack/ironic: Use metadata.create_all() to initialise DB schema https://review.openstack.org/107629 | 12:52 |
| *** igor has joined #openstack-ironic | 12:52 | |
| *** lucas-hungry is now known as lucasagomes | 12:52 | |
| *** igor has quit IRC | 12:52 | |
| viktors | dtantsur: see ^^ | 12:53 |
| *** igor has joined #openstack-ironic | 12:53 | |
| * dtantsur is looking | 12:53 | |
| *** rameshg87 has quit IRC | 12:53 | |
| *** yuanying has joined #openstack-ironic | 12:54 | |
| dtantsur | viktors, I like it. please feel free to raise base.py change on top of it | 12:55 |
| dtantsur | lucasagomes, please have a look/approve too ^^^ | 12:55 |
| lucasagomes | dtantsur, ack will do now | 12:55 |
| *** faizan has joined #openstack-ironic | 12:56 | |
| *** igor has quit IRC | 12:56 | |
| lucasagomes | viktors, dtantsur ack yeah if this changes can be separated I think it makes sense to have the two patches and discuss them individually | 12:56 |
| lucasagomes | just approved the 107629 | 12:57 |
| viktors | lucasagomes, dtantsur - ok, I'll try to do something with DB testing :) | 12:58 |
| lucasagomes | viktors, ta much! | 12:58 |
| viktors | lucasagomes: thank you! | 12:58 |
| *** enikanorov has joined #openstack-ironic | 13:01 | |
| *** rloo has joined #openstack-ironic | 13:02 | |
| viktors | by the way, is there any volunteers to fix documentation for ironic installation and close bug 1347604 ? :) | 13:02 |
| *** yuanying has quit IRC | 13:03 | |
| openstackgerrit | Yuriy Zveryanskyy proposed a change to openstack/ironic: Control extra space for images conversion in image_cache https://review.openstack.org/115974 | 13:07 |
| *** rloo has quit IRC | 13:08 | |
| *** rloo has joined #openstack-ironic | 13:08 | |
| *** rloo has quit IRC | 13:08 | |
| *** rloo has joined #openstack-ironic | 13:09 | |
| *** rloo has quit IRC | 13:09 | |
| *** rloo has joined #openstack-ironic | 13:10 | |
| *** rloo_ has joined #openstack-ironic | 13:11 | |
| dtantsur | lucasagomes, mind having one more look ^^^? | 13:13 |
| *** rloo_ has quit IRC | 13:13 | |
| *** linggao has joined #openstack-ironic | 13:13 | |
| dtantsur | ah I see | 13:14 |
| *** rloo_ has joined #openstack-ironic | 13:15 | |
| *** rloo has quit IRC | 13:16 | |
| *** rloo_ has quit IRC | 13:16 | |
| *** enikanorov__ has joined #openstack-ironic | 13:17 | |
| *** rloo has joined #openstack-ironic | 13:19 | |
| *** enikanorov has quit IRC | 13:19 | |
| rloo | hi Ironickers | 13:20 |
| rloo | lucasagomes: when you have a minute, I added a comment to 118397 but Shrews already approved. Want to make sure you see it. | 13:21 |
| Shrews | rloo: oh, was the comment there before i approved? | 13:21 |
| rloo | Shrews: no, I'm just too slow. You approved before I added the comment, but I think it is worth getting an answer to my question. | 13:22 |
| Shrews | rloo: ah. sorry about that | 13:22 |
| lucasagomes | rloo, will do | 13:22 |
| lucasagomes | dtantsur, did already | 13:23 |
| rloo | Shrews: no worries. It is great that we have so many people reviewing at the same time :D | 13:23 |
| dtantsur | rloo, hi | 13:24 |
| rloo | hi dtantsur | 13:24 |
| *** rameshg87 has joined #openstack-ironic | 13:24 | |
| lucasagomes | rloo, yeah :( as I'm using the default dialect there I think we are safe | 13:24 |
| lucasagomes | but yeah it would be good to have that catch thre | 13:24 |
| lucasagomes | there* | 13:24 |
| rloo | lucasagomes: ok, do you want me to open a bug about it? | 13:25 |
| rameshg87 | lucasagomes: rloo: please have a look at ilo-virtual-media driver review: https://review.openstack.org/113865 | 13:25 |
| lucasagomes | rloo, hmm I can toss a patch up for that real quick, idk if we need a bug for it now (as we use the default there, it should never raise that exception) | 13:26 |
| lucasagomes | rloo, that works? | 13:26 |
| rloo | lucasagomes: i'm fine if you toss a patch up. Wasn't sure you were going to ;) | 13:26 |
| lucasagomes | rloo, ack, well I've the code already open here cause I was doing that changes | 13:26 |
| lucasagomes | 1 sec | 13:26 |
| rloo | rameshg87: I've got a whole pile of patches to review. I decided to go on strike and not review any more til kilo. (just joking) | 13:27 |
| rameshg87 | rloo: :) | 13:27 |
| rameshg87 | rloo: you could just complete the patches that you started reviewing :D | 13:27 |
| rloo | rameshg87: no, I don't always do that cuz I don't work every day and I don't want things delayed because I'm not around. Also, it takes me a long time to do a review and I find it hard to iterate too many times on the same patch. | 13:28 |
| rameshg87 | rloo: okay :) no problems | 13:29 |
| rloo | rameshg87: I suppose I could just select a small number of patches instead and only focus on those. | 13:29 |
| rameshg87 | rloo: okay | 13:30 |
| *** bluex-pl has joined #openstack-ironic | 13:34 | |
| *** jasondotstar has joined #openstack-ironic | 13:39 | |
| jroll | morning lucasagomes dtantsur Shrews rloo rameshg87 bluex-pl and the rest of ironic :) | 13:41 |
| jroll | (busy day?) | 13:41 |
| lucasagomes | jroll, morning | 13:41 |
| rloo | morning jroll | 13:41 |
| dtantsur | jroll, morning | 13:41 |
| rloo | rameshg87: wrt 113865. https://review.openstack.org/#/c/113865/23/ironic/drivers/modules/ilo/common.py | 13:44 |
| rloo | rameshg87: line 305 | 13:44 |
| rloo | rameshg87: what I dont' like is the '6' | 13:44 |
| jroll | bluex-pl: read that comment, can you propose that as a new version of your patch? don't worry about tests yet, but I want to see what it looks like | 13:44 |
| jroll | bluex-pl: I expect that might be an ok solution | 13:44 |
| rloo | rameshg87: if the string changes from 'swift:' to 'foo:', then it becomes 4, not 6. IF someone remembers to change that number. | 13:45 |
| rloo | rameshg87: so if you split boot_iso into key, value, you can check key==swift, and just use value. | 13:45 |
| *** faizan has quit IRC | 13:46 | |
| jroll | faizan nooooooo don't go :( | 13:46 |
| *** r-daneel has joined #openstack-ironic | 13:47 | |
| openstackgerrit | A change was merged to openstack/ironic: Fix minor issues in the DRAC driver https://review.openstack.org/118397 | 13:47 |
| jroll | gah uefi is so ready | 13:49 |
| Shrews | jroll: how ready is it? .... | 13:49 |
| jroll | rloo: there's one nit here, but will you review this to make sure your concerns are taken care of? https://review.openstack.org/#/c/114357/ | 13:49 |
| jroll | other than that lgtm | 13:49 |
| jroll | Shrews: needs one little nit fixed, I might just do it | 13:50 |
| Shrews | jroll: oh, i thought that was the opening line to a uefi joke :-P | 13:50 |
| rloo | jroll: I'm starting to think that if things are almost ready, I shouldn't look. | 13:50 |
| jroll | lol | 13:50 |
| jroll | ^ for both of you :P | 13:50 |
| rameshg87 | jroll: good morning | 13:55 |
| jroll | heya rameshg87 :) | 13:55 |
| *** chuckC has quit IRC | 13:55 | |
| rameshg87 | rloo: just reading your comments | 13:55 |
| *** dhellmann has quit IRC | 13:57 | |
| dtantsur | jroll, left more comments there | 13:57 |
| rloo | rameshg87: i started reviewing your patch, but jroll asked me to look at uefi so yours is in the queue again. | 13:57 |
| rameshg87 | rloo: i can change if you really insist, but my defense was it was all internally generated within the code. so splitting them into key-value pair was just a overhead :) | 13:58 |
| *** dhellmann has joined #openstack-ironic | 13:58 | |
| rameshg87 | rloo: there is no chance of us getting 'foo' in there unless someone writes new code without reading the current code | 13:58 |
| openstackgerrit | Lucas Alvares Gomes proposed a change to openstack/ironic: Handle DracInvalidFilterDialect exception https://review.openstack.org/118638 | 13:59 |
| lucasagomes | rloo, ^ | 13:59 |
| rameshg87 | rloo: no problem. | 13:59 |
| rloo | rameshg87: so it doesn't have to be split, that was a suggestion. I just don't like the hardcoded 6. you can even do 'len('swift:') or whatever. | 13:59 |
| rloo | lucasagomes: thx | 13:59 |
| *** romcheg1 has quit IRC | 13:59 | |
| *** yuanying has joined #openstack-ironic | 14:00 | |
| jroll | dtantsur: ok, you think I should push a new version or wait for faizan? want to land this today | 14:00 |
| jroll | lucasagomes: ^^ | 14:00 |
| *** romcheg1 has joined #openstack-ironic | 14:00 | |
| rameshg87 | rloo: ah okay, i get it. i am okay with changing that. | 14:00 |
| lucasagomes | jroll, +1 for landing it today | 14:00 |
| jroll | ok | 14:00 |
| * jroll fixes things | 14:00 | |
| lucasagomes | I think it's mostly nits now, so I don't see a problem in pushing a new quick patch-set for it | 14:01 |
| rameshg87 | rloo: len('swift:') i will change that ... | 14:01 |
| lucasagomes | jroll, thx! | 14:01 |
| dtantsur | jroll, I think it's ok for you to fix | 14:01 |
| rameshg87 | lucasagomes: jroll: is mine ilo-virtual-media-iscsi in your queue ? :) | 14:01 |
| lucasagomes | rameshg87, yes! | 14:01 |
| rameshg87 | rloo: i will wait for you if you have any more comments. if only nits, i will make sure, i will fix it in the next patch. is that fine ? | 14:02 |
| rloo | rameshg87: you realize, you won't like it if you have 'swift:' in two places. similarly for 'glance:' | 14:02 |
| dtantsur | everything targeted to J3 is a priority today | 14:02 |
| rloo | rameshg87: yeah, that's fine. it may take me another hour or 2 to get through yours. | 14:03 |
| rameshg87 | rloo: yeah, okay :(. if it looks too ugly, i will resort to key-value pair. | 14:03 |
| *** sanek11 has quit IRC | 14:03 | |
| *** lazy_prince is now known as killer_prince | 14:03 | |
| *** pcrews has joined #openstack-ironic | 14:04 | |
| openstackgerrit | Stig Telfer proposed a change to openstack/ironic: Adds SNMP power driver https://review.openstack.org/118404 | 14:06 |
| dtantsur | rloo, lucasagomes, do we really need to catch DracInvalidFilterDialect? the only possible cause is programming error IMO | 14:07 |
| *** yuanying has quit IRC | 14:08 | |
| lucasagomes | dtantsur, yeah, that's true... it's super unlikable to happen... I just put it there cause it's a valid exception that can be raised by that method | 14:08 |
| lucasagomes | but yeah, dtantsur to be honest I'm happy handling or not handling it really | 14:09 |
| rloo | dtantsur, lucasagomes: that's the question I had for you. Wanted to make sure that it had been thought of, instead of forgotten to think about it. | 14:09 |
| rloo | dtantsur, lucasagomes: if that made sense... | 14:09 |
| lucasagomes | rloo, oh | 14:09 |
| lucasagomes | rloo, right... hmm yeah as dmitry pointed I don't see how that can be raised if not by an mistake when coding | 14:10 |
| dtantsur | rloo, for me it's just yet another possible programming error, like ValueError | 14:10 |
| rloo | dtantsur, lucasagomes: ie, if it can only be via program error, then maybe you want it to be raised | 14:10 |
| lucasagomes | rloo, I will abandon that patch | 14:10 |
| rloo | dtantsur: sometimes ValueError needs to be caught. It depends... | 14:10 |
| rloo | lucasagomes: that's fine. And I won't look to see if the docstring needs to be updated with "raises..." | 14:11 |
| lucasagomes | rloo, oh there's that too | 14:11 |
| * lucasagomes thinking | 14:11 | |
| *** stelfer has quit IRC | 14:11 | |
| lucasagomes | let's leave it there and see what other folks think about it | 14:11 |
| * rloo hates handling errors etc | 14:12 | |
| * lucasagomes brain's not working properly | 14:12 | |
| NobodyCam | good morning Ironic | 14:17 |
| rloo | morning NobodyCam | 14:18 |
| openstackgerrit | Jim Rollenhagen proposed a change to openstack/ironic: Add UEFI based deployment support in Ironic https://review.openstack.org/114357 | 14:18 |
| jroll | morning NobodyCam :) | 14:18 |
| NobodyCam | morning rloo jroll | 14:18 |
| lucasagomes | morning NobodyCam | 14:18 |
| NobodyCam | morning lucasagomes :) | 14:21 |
| NobodyCam | whats left to land, and can we land it | 14:21 |
| NobodyCam | lol :) | 14:21 |
| jroll | lucasagomes dtantsur NobodyCam https://review.openstack.org/114357 | 14:22 |
| jroll | I think that's ready | 14:22 |
| dtantsur | already looking :) | 14:22 |
| jroll | still don't love the bit about setting boot device, but I don't think there's much more we can do :( | 14:22 |
| romcheg1 | Morning NobodyCam, jroll, rloo! | 14:22 |
| jroll | hiya romcheg1 :) | 14:22 |
| rloo | hi romcheg1 | 14:22 |
| rloo | jroll: wrt 114357, pxe_utils.py, _link_ip_address_pxe_configs(). still want to know what happens if _get_pxe_ip_address_path() raises InvalidIPv4Address. | 14:23 |
| Shrews | lucasagomes: rloo: dtantsur: my view is that exception handling should be done for runtime errors, not programming errors | 14:23 |
| Shrews | so, i'd leave it as is, but that's only my opinion | 14:24 |
| openstackgerrit | Stig Telfer proposed a change to openstack/ironic: Adds SNMP power driver https://review.openstack.org/118404 | 14:24 |
| jroll | rloo: oops, I missed that | 14:24 |
| jroll | rloo: what I wanted to do, was validate the ip address when we get it from neutron | 14:25 |
| jroll | rloo: and don't return it in get_ip_addresses if it is not valid | 14:25 |
| jroll | how does that sound? | 14:25 |
| rloo | jroll: that's fine with me. what happens if/when we handle ipv6? | 14:26 |
| jroll | lord. | 14:26 |
| rloo | jroll: just update the code in neutron then. forget i asked. | 14:26 |
| jroll | honestly, I doubt elilo supports it | 14:26 |
| jroll | but yeah | 14:26 |
| lucasagomes | Shrews, thanks, yeah I'm grand leaving as is | 14:26 |
| lucasagomes | I will abandon that patch, the more I think the less sense it makes | 14:26 |
| rloo | lucasagomes, Shrews: yeah, I'm fine with that ;) | 14:26 |
| dtantsur | jroll, once rloo's concern is fixed, I'm ok with the code | 14:26 |
| jroll | ok | 14:27 |
| jroll | sec | 14:27 |
| lucasagomes | rloo, Shrews dtantsur thanks | 14:27 |
| openstackgerrit | Szymon Wróblewski proposed a change to openstack/ironic-python-agent: Remove unnecessary argument injection in agent sync and async decorators. https://review.openstack.org/117529 | 14:27 |
| *** rushiagr is now known as rushiagr_away | 14:28 | |
| toabctl | GheRivero: hey. friendly reminder for another review of https://review.openstack.org/#/c/117704/ | 14:32 |
| *** jcoufal_ has joined #openstack-ironic | 14:32 | |
| *** jcoufal has quit IRC | 14:34 | |
| bluex-pl | jroll: finally found some time to push it, take a look when you can | 14:34 |
| jroll | bluex-pl: thanks, will look later on | 14:35 |
| jroll | russell_h: can you look at this as well? https://review.openstack.org/117529 | 14:35 |
| *** HenryG_afk is now known as HenryG | 14:36 | |
| jroll | bluex-pl: it is kind of duplicating the name between command_map and the decorator, but idk, might be good | 14:36 |
| rloo | jroll: any idea if uefi would be used with ipminative? | 14:37 |
| *** chuckC has joined #openstack-ironic | 14:38 | |
| jroll | rloo: good question, does it matter? :) | 14:38 |
| jroll | rloo: there's the whole thing with... nobody is using ipminative | 14:39 |
| rloo | jroll: yes, but I decided that someone can change the comments/msg in that case. | 14:39 |
| rloo | jroll: asking cuz 'ipmitool' is mentioned in the patch. but forget about it. | 14:39 |
| jroll | mmm yeah, I would assume ipminative behaves the same way | 14:39 |
| jroll | so maybe that's bad... | 14:40 |
| rloo | it is bad, when/if it happens. so better to try to address now, but i know we're in a hurry. | 14:40 |
| jroll | I mean, I can push more | 14:40 |
| jroll | about to push another anyhow for the invalid ip thing | 14:40 |
| jroll | I can just s/ipmitool/ipmi/ there I think? | 14:41 |
| rloo | jroll: ok, if you don't mind. | 14:41 |
| *** rameshg87 has quit IRC | 14:41 | |
| rloo | jroll: yeah. if some other mgt interface is used in the future, someone else can update it further. but at least it'll be ok for ipmi. | 14:41 |
| openstackgerrit | Jim Rollenhagen proposed a change to openstack/ironic: Add UEFI based deployment support in Ironic https://review.openstack.org/114357 | 14:41 |
| jroll | yeah | 14:41 |
| jroll | I would expect that other management interfaces will work ok | 14:42 |
| rloo | jroll: and that's it for my comments. will wait for your next patch. | 14:42 |
| jroll | I don't love having references to ipmi in the pxe driver code, but if only ipmi fails, I think it makes sense | 14:42 |
| jroll | rloo: just pushed :P | 14:42 |
| rloo | jroll: you're fast! | 14:42 |
| jroll | I was looking at it after you asked :P | 14:43 |
| jroll | was waiting for tests to run | 14:43 |
| *** romcheg1 has left #openstack-ironic | 14:43 | |
| *** romcheg1 has joined #openstack-ironic | 14:44 | |
| rloo | jroll: I don't actually like raising ConfigInvalid exception from a validate() because I'm afraid of the ramifications (eg, code that handles Invalid/Missing might now have to be updated to handle ConfigInvalid). | 14:45 |
| rloo | jroll: sorry, I should just shut up. | 14:45 |
| jroll | lol | 14:45 |
| jroll | I mean... that's a good point | 14:45 |
| jroll | what would you prefer? invalidparameter whatever its called? | 14:46 |
| jroll | I'm almost wondering if that's the right thing to do | 14:46 |
| rloo | jroll: I think Invalid would be easier to deal with. | 14:46 |
| jroll | if one is running with ipxe enabled, they shouldn't configure a node that way | 14:46 |
| rloo | jroll: or at least an exception that is subclassed from Invalid. | 14:46 |
| jroll | hrm | 14:47 |
| rloo | jroll: it does bother me that ipxe is enabled 'globally', but uefi is enabled node by node only. | 14:47 |
| jroll | right... I was just thinking about that | 14:47 |
| jroll | but ipxe is more of an environmental thing, I think | 14:48 |
| jroll | I can't imagine why one would want half ipxe, half not-ipxe | 14:48 |
| rloo | which is what made me think that a node's settings maybe overrides an environmental setting. | 14:48 |
| jroll | (actually, can't imagine why one would want anything non-ipxe, but that's a different topic) | 14:48 |
| jroll | mmm | 14:48 |
| rloo | but that could be a separate patch. | 14:49 |
| jroll | so drop to "regular pxe" if the node is configured for uefi? | 14:49 |
| rloo | yeah. | 14:49 |
| jroll | interesting | 14:49 |
| jroll | that should definitely be a separate patch | 14:49 |
| rloo | if that does happen, then we wouldn't want to raise any exception cuz there wouldn't be a conflict, so seems safer to use InvalidP.. instead of ConfigInvalid | 14:50 |
| jroll | yeah; agree | 14:51 |
| *** rameshg87 has joined #openstack-ironic | 14:51 | |
| jroll | there we are | 14:52 |
| openstackgerrit | Jim Rollenhagen proposed a change to openstack/ironic: Add UEFI based deployment support in Ironic https://review.openstack.org/114357 | 14:52 |
| * jroll brb | 14:52 | |
| *** jcoufal_ has quit IRC | 14:55 | |
| *** rushiagr_away is now known as rushiagr | 14:59 | |
| *** yuanying has joined #openstack-ironic | 15:04 | |
| bluex-pl | jroll: yeah, I think decorators could update command_map automatically (http://paste.openstack.org/show/105253/) | 15:05 |
| *** bluex-pl has quit IRC | 15:11 | |
| jroll | neat | 15:11 |
| *** yuanying has quit IRC | 15:14 | |
| *** jcoufal has joined #openstack-ironic | 15:22 | |
| *** rloo has quit IRC | 15:29 | |
| *** rloo has joined #openstack-ironic | 15:29 | |
| NobodyCam | brb | 15:31 |
| *** rameshg871 has joined #openstack-ironic | 15:38 | |
| rameshg871 | rloo: i am just fixing the issue straightaway along with nits from dtantsur | 15:38 |
| *** bmahalakshmi has joined #openstack-ironic | 15:40 | |
| *** rameshg87 has quit IRC | 15:40 | |
| openstackgerrit | Ramakrishnan G proposed a change to openstack/ironic: IloVirtualMediaIscsi deploy driver https://review.openstack.org/113865 | 15:41 |
| *** rameshg87 has joined #openstack-ironic | 15:41 | |
| *** rameshg871 has quit IRC | 15:43 | |
| *** chuckC has quit IRC | 15:45 | |
| *** bandicot has joined #openstack-ironic | 15:45 | |
| *** Poornima has quit IRC | 15:49 | |
| *** chuckC has joined #openstack-ironic | 15:53 | |
| *** igordcard has joined #openstack-ironic | 15:53 | |
| *** foexle has quit IRC | 15:56 | |
| *** eghobo has joined #openstack-ironic | 15:56 | |
| dtantsur | calling it a day, see you | 15:57 |
| rloo | bye dtantsur | 15:58 |
| Shrews | goodnight dtantsur | 15:58 |
| romcheg1 | Bye dtantsur! | 15:58 |
| *** dtantsur is now known as dtantsur|afk | 15:58 | |
| NobodyCam | night dtantsur|afk | 15:59 |
| *** sirushti_ has joined #openstack-ironic | 16:03 | |
| *** pelix1 has joined #openstack-ironic | 16:04 | |
| *** ndipanov has quit IRC | 16:05 | |
| *** victor_lowther_ has joined #openstack-ironic | 16:07 | |
| *** NobodyCa1 has joined #openstack-ironic | 16:07 | |
| *** sirushti has quit IRC | 16:08 | |
| *** pelix has quit IRC | 16:08 | |
| *** MattMan has quit IRC | 16:08 | |
| *** bandicot has quit IRC | 16:08 | |
| *** jasondotstar has quit IRC | 16:08 | |
| *** k4n0 has quit IRC | 16:08 | |
| *** dividebin has quit IRC | 16:08 | |
| *** rushiagr has quit IRC | 16:08 | |
| *** victor_lowther has quit IRC | 16:08 | |
| *** NobodyCam has quit IRC | 16:08 | |
| *** krtaylor has quit IRC | 16:08 | |
| *** GheRivero has quit IRC | 16:08 | |
| *** dividehex has joined #openstack-ironic | 16:08 | |
| *** Ugallu has joined #openstack-ironic | 16:08 | |
| *** sirushti_ is now known as sirushti | 16:08 | |
| *** k4n0_ has joined #openstack-ironic | 16:09 | |
| *** rushiagr has joined #openstack-ironic | 16:10 | |
| *** yuanying has joined #openstack-ironic | 16:10 | |
| *** MattMan has joined #openstack-ironic | 16:10 | |
| *** krtaylor has joined #openstack-ironic | 16:10 | |
| *** igordcard has quit IRC | 16:10 | |
| *** krtaylor has quit IRC | 16:11 | |
| *** GheRivero has joined #openstack-ironic | 16:13 | |
| *** scubacuda has joined #openstack-ironic | 16:15 | |
| *** victor_lowther_ is now known as victor_lowther | 16:15 | |
| *** romcheg2 has joined #openstack-ironic | 16:16 | |
| *** romcheg1 has quit IRC | 16:17 | |
| *** yuanying has quit IRC | 16:18 | |
| NobodyCa1 | so 114357 is good to land after mr approves? | 16:20 |
| *** dlaube has joined #openstack-ironic | 16:21 | |
| JayF | I think so, it's been through quite the review gamut :) | 16:21 |
| NobodyCa1 | lol and 4 +2s at this point :) | 16:23 |
| *** NobodyCa1 is now known as NobodyCam | 16:24 | |
| NobodyCam | just the tempest test left to go | 16:26 |
| *** igordcard has joined #openstack-ironic | 16:32 | |
| NobodyCam | brb | 16:33 |
| *** romcheg1 has joined #openstack-ironic | 16:33 | |
| *** romcheg2 has quit IRC | 16:35 | |
| *** jistr has quit IRC | 16:35 | |
| *** krtaylor has joined #openstack-ironic | 16:37 | |
| *** igordcard has quit IRC | 16:40 | |
| *** romcheg1 has left #openstack-ironic | 16:40 | |
| NobodyCam | romcheg: still fix'n up packages? | 16:40 |
| *** romcheg1 has joined #openstack-ironic | 16:42 | |
| NobodyCam | rameshg87: you around? | 16:42 |
| rameshg87 | NobodyCam: yes | 16:44 |
| rameshg87 | NobodyCam: i was about to ping you :-) | 16:44 |
| NobodyCam | hehehe :) rameshg87 up for a rebase on https://review.openstack.org/#/c/115885? | 16:44 |
| rameshg87 | NobodyCam: yes, but waiting for the parent to get approved so that i can avoid more rebasing :) | 16:45 |
| rameshg87 | NobodyCam: shall i rebase it again now ? | 16:45 |
| NobodyCam | let /me take a look at dep now | 16:46 |
| rameshg87 | NobodyCam: okay | 16:47 |
| rameshg87 | JayF: would you also like to take a look at https://review.openstack.org/#/c/113865/ | 16:49 |
| JayF | rameshg87: I have a vote on it :) | 16:49 |
| rameshg87 | JayF: for a +2 :) | 16:49 |
| JayF | I am not a core on Ironic | 16:49 |
| JayF | only Ironic-specs and Ironic-python-agent | 16:49 |
| rameshg87 | JayF: oh okay :) | 16:50 |
| rameshg87 | lucasagomes: still around for a review on https://review.openstack.org/#/c/113865/ | 16:51 |
| JayF | I think we'll all be looking at that today :) | 16:52 |
| Shrews | JayF: your comment on 118467 about separating power states... you mean put them in another file? Or separating them visually within the produced documentation? | 16:52 |
| JayF | it's gotta land for j-3 | 16:52 |
| JayF | Shrews: yes. | 16:52 |
| JayF | Shrews: one or the other | 16:52 |
| JayF | Shrews: just as it is now it's all mixed together | 16:52 |
| Shrews | JayF: i can't find a way to manipulate docstrings to do it within a single module | 16:53 |
| JayF | what's the review # for that? | 16:53 |
| Shrews | anyway, that's low priority | 16:53 |
| Shrews | 118467 | 16:53 |
| JayF | Shrews: even whitespace, or something would be helpful | 16:54 |
| JayF | Shrews: like # NOTE(shrews) All states below here are power states | 16:54 |
| jroll | Shrews: I'd argue we should not make any actual code changes there... just to not touch the nova reviews | 16:54 |
| JayF | jroll: I have that comment on there already :) | 16:55 |
| Shrews | jroll: yeah, well that's why it's WIP'd | 16:55 |
| jroll | also, I think init may come in useful one day | 16:55 |
| jroll | right | 16:55 |
| Shrews | it was just bugging me | 16:55 |
| jroll | JayF: right, I think I worded it different :p this shpuld just be docs/comments changes IMO | 16:56 |
| rameshg87 | NobodyCam: i am just rebasing 115885 anyway , hoping the parent will go through :) | 16:56 |
| rameshg87 | jroll: got some time for a review on https://review.openstack.org/#/c/113865/ ? :) | 16:57 |
| jroll | rameshg87: I'm on mobile right now but will look later today :) | 16:58 |
| openstackgerrit | Ramakrishnan G proposed a change to openstack/ironic: IloVirtualMediaAgent deploy driver https://review.openstack.org/115885 | 16:58 |
| jroll | rameshg87: I want to land all the ilo stuff today, will make it happen | 16:58 |
| jroll | rameshg87: you okay with us fixing nits in those if you aren't around? | 16:59 |
| *** rameshg871 has joined #openstack-ironic | 16:59 | |
| jroll | rameshg87: I'm on mobile right now but will look later today :) | 17:00 |
| jroll | rameshg87: I want to land all the ilo stuff today, will make it happen | 17:00 |
| jroll | rameshg87: you okay with us fixing nits in those if you aren't around? | 17:00 |
| openstackgerrit | Lucas Alvares Gomes proposed a change to openstack/ironic: Driver merge review comments from 111425 https://review.openstack.org/118693 | 17:01 |
| *** derekh has quit IRC | 17:01 | |
| lucasagomes | ^^ folks | 17:01 |
| lucasagomes | rameshg87, will do | 17:01 |
| rameshg871 | lucasagomes: thanks :) | 17:01 |
| *** rameshg87 has quit IRC | 17:01 | |
| rloo | lucasagomes: I'm looking at 111425 now. Already have a nit :-( | 17:03 |
| *** jcoufal has quit IRC | 17:03 | |
| lucasagomes | rloo, cool, post there and I can update that ironic patch | 17:03 |
| lucasagomes | actually if you can please update that patch too | 17:03 |
| lucasagomes | just put a new patch-set fixing the nit | 17:04 |
| rloo | lucasagomes: ok, i only started looking and am eating lunch at the same time so may take a bit of time ... | 17:04 |
| lucasagomes | rloo, no bothers, we may also have to talk to mrda to sync it over | 17:04 |
| lucasagomes | so it's cool | 17:04 |
| rloo | lucasagomes: okay | 17:04 |
| *** yuanying has joined #openstack-ironic | 17:15 | |
| rloo | hey lucasagomes or anyone, question about the driver: https://review.openstack.org/#/c/111425/19/nova/virt/ironic/driver.py | 17:15 |
| rloo | in _validate_instance_and_node() | 17:16 |
| rloo | it just says that the instance exists if it returns true, doesn't it? I don't see where the 'node' comes in. | 17:16 |
| *** chuckC has quit IRC | 17:16 | |
| *** MattMan has quit IRC | 17:17 | |
| rloo | ^^ NobodyCam, do you know? | 17:17 |
| lucasagomes | rloo, you mean in the instance_exists() ? | 17:18 |
| lucasagomes | yeah the docstring is not super well worded | 17:18 |
| rloo | lucasagomes: no, I mean the _validate_instance_and_node() method. | 17:18 |
| lucasagomes | rloo, that checks if the instance uuid is already associated with some node in Ironic | 17:19 |
| rloo | lucasagomes: it just looks like it validates that there is *a* node associated with that instance. | 17:19 |
| lucasagomes | yup | 17:19 |
| *** pelix1 is now known as pelix | 17:19 | |
| rloo | lucasagomes: but the docstring seems to indicate something else. that it is associated with 'this node'? | 17:19 |
| lucasagomes | that reruns the node object as well | 17:20 |
| lucasagomes | returns | 17:20 |
| lucasagomes | rloo, oh... I see hmm | 17:20 |
| *** harlowja_away is now known as harlowja | 17:20 | |
| lucasagomes | this node makes no sense because that's the driver | 17:21 |
| lucasagomes | the node uuid actually is _in_ the instance object | 17:21 |
| lucasagomes | I mean... urgh... | 17:22 |
| rloo | lucasagomes: but nothing here checks that the node's uuid (in the instance object) is the same uuid as the returned node | 17:22 |
| lucasagomes | rloo, yeah... AFAIK instance has an attribute 'node' that is the node uuid | 17:22 |
| lucasagomes | so it may make sense to check whether it's same | 17:22 |
| lucasagomes | :( | 17:23 |
| lucasagomes | and ofc reword that docstring | 17:23 |
| *** yuanying has quit IRC | 17:23 | |
| rloo | i don't really want to change code at this point. let me see who/what calls this method, etc... | 17:23 |
| rloo | so nothing checks that the node's uuid is the same uuid as the instance thinks it should be. | 17:25 |
| *** stelfer has joined #openstack-ironic | 17:26 | |
| lucasagomes | rloo, yeah, I think it makes sense to check that too... maybe that worth open a bug | 17:26 |
| lucasagomes | because InstanceNotFound may not be the appropriated error for that case | 17:27 |
| lucasagomes | the instance exists the node is not correct | 17:27 |
| lucasagomes | gotta take a deeper look into it | 17:27 |
| rloo | lucasagomes: so should I add a comment about it in the patch, or pretend I didn't see it? | 17:27 |
| lucasagomes | rloo, hah hmmmm | 17:29 |
| * lucasagomes thinking | 17:29 | |
| lucasagomes | rloo, I really dunno... well I think we should be transparent | 17:29 |
| lucasagomes | maybe put a comment and say that we will open a bug about it | 17:29 |
| rloo | or should i wait and ask mrda? i can put a comment and then we can put another comment saying it'll be fixed in another patch? | 17:29 |
| lucasagomes | I don't even know if that case is possible without investigating it | 17:29 |
| lucasagomes | but we have to be transparent in the reviews and put our concerns there IMO | 17:30 |
| rloo | or we can update the docstring to reflect what it is really doing. | 17:30 |
| rloo | sec, let me see if i can figure out who wrote that method. maybe they will know... | 17:30 |
| lucasagomes | rloo, yeah also | 17:30 |
| lucasagomes | rloo, you can put the docstring changes on that patch I put up fixing the other problem with 111425 | 17:30 |
| rloo | eww, i have to fix the docstring? :-) | 17:31 |
| lucasagomes | lol | 17:31 |
| lucasagomes | rloo, please? | 17:31 |
| rloo | ha ha. yeah, after i finish going through the patch. | 17:32 |
| lucasagomes | ack | 17:32 |
| lucasagomes | because I'm almost calling it a day here | 17:32 |
| lucasagomes | it's late and I gotta go out buy some food for dinner | 17:32 |
| rloo | lucasagomes: yeah, go home! | 17:33 |
| lucasagomes | alright yeah... I'm going to call it a day! | 17:35 |
| lucasagomes | have a good night everybody! | 17:35 |
| lucasagomes | see ye tomorrow | 17:35 |
| rloo | goodnight lucasagomes | 17:35 |
| *** stelfer has quit IRC | 17:37 | |
| *** lucasagomes is now known as lucas-dinner | 17:37 | |
| NobodyCam | night lucas-dinner | 17:43 |
| NobodyCam | lucas-dinner: I'm going to land 113865 | 17:44 |
| NobodyCam | rameshg871: 113865 has several nit's could / would you be willing to address them in a new patch? | 17:45 |
| lucas-dinner | NobodyCam, ack | 17:46 |
| NobodyCam | :) | 17:46 |
| *** pcrews has quit IRC | 17:47 | |
| rameshg871 | NobodyCam: i can address them ... | 17:48 |
| rameshg871 | NobodyCam: alternatively i could address them 115885 as well if you like :) | 17:48 |
| rameshg871 | NobodyCam: the patch which is dependent on it | 17:49 |
| rameshg871 | lucas-dinner: regarding moving to vendor.py, the intention was to keep the related vendor passthrus together. currently we have the vendor-passthru for deploy which is in deploy.py | 17:50 |
| NobodyCam | rameshg871: up to you. but I'm ok with landing 113865 as is, if you can address the nit in 115885 or a new patch I'm ok with that too | 17:51 |
| rameshg871 | NobodyCam: i will address them in 115885 itself | 17:52 |
| NobodyCam | then I am landing 113865 now !!!! | 17:52 |
| rameshg871 | NobodyCam: thanks ... i will start addressing them in 115885 then ... | 17:52 |
| NobodyCam | +a'd :) | 17:53 |
| rameshg871 | NobodyCam: thanks :) | 17:54 |
| openstackgerrit | A change was merged to openstack/ironic: Control extra space for images conversion in image_cache https://review.openstack.org/115974 | 17:55 |
| Shrews | NobodyCam: nooooooo | 17:55 |
| * Shrews too slow | 17:55 | |
| NobodyCam | huh | 17:59 |
| NobodyCam | what | 17:59 |
| NobodyCam | wait | 17:59 |
| Shrews | rameshg871: i left additional comments on 113865 for you. now i know how rloo felt this morning when I approved a review out from under her :) | 17:59 |
| NobodyCam | :( | 17:59 |
| Shrews | NobodyCam: no worries. all stuff that can be addressed later. | 17:59 |
| NobodyCam | :) | 17:59 |
| NobodyCam | phew | 17:59 |
| NobodyCam | jroll: JayF: we (I) will need lots of your help with 115885, for the agent side of things | 18:02 |
| jroll | NobodyCam: hello hello, I can help | 18:03 |
| NobodyCam | morning jroll :) have some coffee | 18:04 |
| NobodyCam | lol | 18:04 |
| jroll | hehe, thank you! | 18:04 |
| NobodyCam | just pointing out that i am not a IPA expert and would like your eyes on 115885!! | 18:04 |
| jroll | yep :) | 18:05 |
| jroll | will review | 18:05 |
| NobodyCam | :) | 18:05 |
| jroll | did we land ilo/iscsi yet/ | 18:05 |
| NobodyCam | ty | 18:05 |
| rameshg871 | Shrews: just looking at it now | 18:05 |
| rameshg871 | Shrews: will address them in follow up patch | 18:05 |
| jroll | oh niiiice | 18:05 |
| jroll | we did | 18:05 |
| jroll | \o/ | 18:05 |
| NobodyCam | :) | 18:05 |
| Shrews | rameshg871++ | 18:05 |
| jroll | JayF: going to want your eyes on 115885 too | 18:05 |
| rloo | Shrews: still good to add comments, even though it is 'too late' :-) | 18:06 |
| JayF | yup I know :) | 18:06 |
| rameshg871 | Shrews: NobodyCam: i have a doubt in general. do we document exceptions for all the functions, even for private internal methods within a file ? :) | 18:06 |
| JayF | was waiting for the one above it to land | 18:06 |
| Shrews | rameshg871: everyone else can correct me if i'm wrong, but if a function can raise any exception (even from other functions it calls), it should be documented. | 18:07 |
| rameshg871 | Shrews: but what if the function doesn't itself raise, but one of the called function raises ? | 18:08 |
| NobodyCam | Shrews: I like to see that.. I have yet to block a patch for not doing it | 18:08 |
| rameshg871 | Shrews: do we document the exceptions of the called functions too ? | 18:08 |
| Shrews | rameshg871: if you aren't handling them, and they can be propogated up, i would think so | 18:09 |
| NobodyCam | ++ | 18:09 |
| Shrews | that way, a programmer hoping to use that function knows they need to handle those exceptions | 18:09 |
| rloo | Shrews, NobodyCam, rameshg871: for noninternal functions (ie, those not prefixed with _), should document all exceptions that can be raised by it (or any called functions). | 18:09 |
| rloo | that's what I was told | 18:10 |
| openstackgerrit | A change was merged to openstack/ironic: Use metadata.create_all() to initialise DB schema https://review.openstack.org/107629 | 18:10 |
| rloo | wrt internal functions. I think they should do the same too, but I was told that they either shouldn't or don't need to. | 18:10 |
| rloo | (I forgot the exact details). | 18:10 |
| rameshg871 | rloo: okay .. | 18:10 |
| Shrews | yeah, i don't think we have it written down anywhere, but i still like to see it | 18:11 |
| rameshg871 | rloo: Shrews: just wondering if we document all the exceptions of all the called methods, won't it pile up the list of exceptions in docstring :) | 18:11 |
| jroll | I think for internal functions, it's not required, I still believe they are nice to have | 18:12 |
| rloo | am thinking I should have tried to capture that info. it is probably in IRC somewhere, a discussion I had with deva I think. many months ago. | 18:12 |
| rloo | rameshg871: if you were a programmer, would you want to know what kind of exceptions might be raised? | 18:12 |
| rloo | rameshg871: sorry, I don't mean to say that you aren't a programmer | 18:12 |
| rameshg871 | rloo: yeah, i get it :) | 18:12 |
| Shrews | rloo: Perhaps we should start compiling an "Ironic Best Dev Practices" wiki? | 18:12 |
| jroll | lol, harsh :P | 18:12 |
| rameshg871 | Shrews: ++ | 18:13 |
| rloo | Shrews, I think that's a great idea. | 18:13 |
| rloo | do we even know when/if we should put docstrings for methods? :-) | 18:14 |
| Shrews | when we get -1'd b/c we don't have it? ;) | 18:14 |
| rameshg871 | Shrews: LOL | 18:14 |
| Shrews | oh yay. that time of day when my laptop starts acting flakey. /me reboots | 18:17 |
| *** yuanying has joined #openstack-ironic | 18:20 | |
| NobodyCam | woo hoo just added a item to the NOVA agenda: https://wiki.openstack.org/wiki/Meetings/Nova#Agenda_for_next_meeting | 18:20 |
| NobodyCam | looks like I need to be up and awake at 7am tomorrow :-p | 18:20 |
| jroll | NobodyCam: we also have our graduation review on the 9th :o | 18:20 |
| NobodyCam | jroll: ya just figured I'd ask how they want us to attempt to land the proxy api commands stuff | 18:21 |
| jroll | yeah | 18:22 |
| jroll | they haven't given you a good answer yet? | 18:22 |
| rloo | will we need a feature freeze exception for the driver? | 18:22 |
| jroll | god I hope not | 18:22 |
| NobodyCam | oh should ask for one just in case or would that take the pressure of the nova cores from reviewing our patches? | 18:23 |
| jroll | should talk to mrda | 18:23 |
| rloo | maybe mrda would know | 18:23 |
| jroll | I think he's been talking to nova about it | 18:23 |
| NobodyCam | yea I think he has been | 18:23 |
| NobodyCam | will hit him up today | 18:24 |
| NobodyCam | I can add it a ffe request to their agenda if needed | 18:24 |
| *** yuanying has quit IRC | 18:28 | |
| *** pcrews has joined #openstack-ironic | 18:28 | |
| jroll | this ipa thing is great | 18:31 |
| jroll | rameshg871: ^^ :) | 18:31 |
| openstackgerrit | Rishabh Jain proposed a change to openstack/ironic: Spelling and Grammar mistakes fixed https://review.openstack.org/118733 | 18:32 |
| *** rushiagr is now known as rushiagr_away | 18:33 | |
| openstackgerrit | linggao proposed a change to openstack/ironic: Interactive console support for ipminative driver https://review.openstack.org/97331 | 18:36 |
| rameshg871 | jroll: thanks :-) | 18:37 |
| rameshg871 | NobodyCam: i think i will need a manual rebase after submission of https://review.openstack.org/#/c/115974/ | 18:38 |
| rameshg871 | NobodyCam: this is for the approved patch ... | 18:39 |
| rameshg871 | NobodyCam: shall i just raise a new patchset after rebasing ? i think jenkins will fail anyway eventually | 18:40 |
| rameshg871 | jroll: ^^ what's your thought on this | 18:41 |
| jroll | are you sure 115974 will land first? | 18:41 |
| rameshg871 | jroll: 115974 has already landed | 18:42 |
| jroll | I mean, merge | 18:42 |
| jroll | oh | 18:42 |
| jroll | it did, hmm | 18:42 |
| jroll | it's up to you if you want to wait and see or just do it | 18:42 |
| jroll | you could do the rebase locally | 18:42 |
| rameshg871 | jroll: i think i will just do it..it's failing for me locally | 18:42 |
| jroll | if you get conflicts, ok | 18:42 |
| jroll | yeah, go for it | 18:42 |
| openstackgerrit | linggao proposed a change to openstack/ironic: Interactive console support for ipminative driver https://review.openstack.org/97331 | 18:42 |
| rameshg871 | jroll: okay .. | 18:42 |
| jroll | rameshg871: please rebase agent patch too, I'm about to +2 I think | 18:43 |
| *** Ugallu has quit IRC | 18:45 | |
| NobodyCam | jroll: LOL... and I can't find any nits. I love it. | 18:50 |
| jroll | :D | 18:50 |
| jroll | I mean | 18:50 |
| jroll | I tried | 18:50 |
| jroll | I wanted to be as good as ruby | 18:50 |
| jroll | but I couldn't find anything I would change | 18:50 |
| NobodyCam | :) | 18:50 |
| NobodyCam | seems I need to do a walkies | 18:52 |
| rameshg871 | jroll: NobodyCam: okay | 18:52 |
| NobodyCam | anyone have a free cycle to rebase https://review.openstack.org/#/c/117499 | 18:52 |
| rameshg871 | NobodyCam: jroll: can i just push the new patch set ? jenkins will stop processing the approved patch set ? | 18:53 |
| jroll | I can in a few, NobodyCam | 18:53 |
| jroll | rameshg871: yep, go ahead | 18:53 |
| rameshg871 | jroll: NobodyCam: i am addressing the nits in it as well :) | 18:53 |
| openstackgerrit | Ramakrishnan G proposed a change to openstack/ironic: IloVirtualMediaIscsi deploy driver https://review.openstack.org/113865 | 18:54 |
| rameshg871 | NobodyCam: jroll: ^^^ | 18:54 |
| rameshg871 | NobodyCam: jroll: please have a look at it | 18:55 |
| rameshg871 | NobodyCam: jroll: rebase + nits addressed | 18:55 |
| * jroll puts it on the end of his 4 review queue | 18:55 | |
| rloo | jroll: I believe that you can be better than me. Just look a bit more closely ;) | 18:57 |
| jroll | rloo: my nose was touching the screen! | 18:57 |
| jroll | can't look closer | 18:57 |
| jroll | :P | 18:57 |
| rloo | jroll: ohh, sorry, you're one of those. You need to back up instead :D | 18:58 |
| jroll | haha | 18:58 |
| *** penick has joined #openstack-ironic | 19:00 | |
| NobodyCam | rameshg871: awesone... will review and approve once Mr jenkins is thru with it | 19:01 |
| *** derekh has joined #openstack-ironic | 19:03 | |
| NobodyCam | lol | 19:04 |
| NobodyCam | Shrews: (and everyone else too ofc) I'm about to +a 114357 last looks? | 19:08 |
| *** derekh has quit IRC | 19:08 | |
| NobodyCam | going .. going ... | 19:10 |
| rloo | NobodyCam, others: question about the driver: https://review.openstack.org/#/c/111425/19/nova/virt/ironic/driver.py | 19:11 |
| jroll | NobodyCam: do it | 19:11 |
| rloo | wrt list_instances() and list_instance_uuids(). Why does the latter do set(instance_uuids)? | 19:11 |
| NobodyCam | gone. | 19:11 |
| JayF | next summit rloo has to give a talk on how to code review | 19:12 |
| NobodyCam | JayF: ++ | 19:12 |
| NobodyCam | :) | 19:12 |
| rloo | JayF: it takes me a long time. I make myself read almost every line. | 19:12 |
| JayF | reading is never my problem | 19:13 |
| JayF | it's always the understanding that gets me ;) | 19:13 |
| rloo | ha ha. I try to understand most of what I read too ;) | 19:13 |
| jroll | rloo: base class docstring says it should return a set :) | 19:14 |
| jroll | oh, no, I should read code before I look | 19:14 |
| rloo | jroll: but it actually ends up returning a list. list_instance_uuids(). | 19:14 |
| jroll | that's a good question | 19:15 |
| rloo | the only thing I can think of, is if you use a set, you want unique, but the uuids should be unique anyway. | 19:15 |
| openstackgerrit | Ramakrishnan G proposed a change to openstack/ironic: IloVirtualMediaAgent deploy driver https://review.openstack.org/115885 | 19:15 |
| jroll | right | 19:15 |
| rloo | anyway, it seems like either they are unique or they aren't. in any case, both methods should be handlign them the same way, no? | 19:15 |
| rloo | and they should be unique unless we're doing it cuz we have cases where the same instance uuid was associated with more than one node. | 19:16 |
| rameshg871 | jroll: ^^ | 19:16 |
| jroll | rameshg871: thanks | 19:16 |
| NobodyCam | humm | 19:16 |
| NobodyCam | rloo: I'm not sure | 19:16 |
| jroll | rloo: yeah, idk, I'm not worried about it at this point, I'll leave that up to other people | 19:16 |
| jroll | rloo: I would let it land, file a bug, then fix it :P | 19:16 |
| rloo | jroll: what is the fix? to be safe and use set() in both cases? ahh, forget it, I'll not say anything for now. see if someone else notices. | 19:17 |
| jroll | rloo: I tend to think that if the instance uuids are not unique, we have bigger problems | 19:18 |
| JayF | UUID collisions are very rare | 19:19 |
| JayF | and would probably break lots more things than just that :) | 19:19 |
| rloo | jroll: true. I just want this code to land! | 19:19 |
| jroll | rloo: although, maybe it's a different case becuase one loops over "nova instances" and one loops over "ironic nodes" | 19:19 |
| jroll | ironic could have one instance associated with multiple nodes (due to bug or race condition) | 19:19 |
| JayF | Isn't the idea to use a UUID so you /don't/ ever have to worry about getting a collission? :) | 19:19 |
| jroll | but nova instances will always have a unique instance uuid | 19:19 |
| jroll | does that make sense? | 19:19 |
| jroll | nova's db won't allow dupplicate uuids | 19:20 |
| rloo | jroll: they're both looping over the nodes from the call to ironic to get the list of nodes | 19:20 |
| jroll | ironic's did allow that in the past, but now it does not | 19:20 |
| jroll | oh. | 19:20 |
| jroll | right | 19:20 |
| jroll | `for i in` tripped me up | 19:20 |
| jroll | idk, it's probably fine for now | 19:21 |
| jroll | (imho) | 19:21 |
| rloo | yeah, so the only thing I can think of, is that Ironic might have had 2+ nodes associated with the same instance uuid. there were issues before wrt deploying an instance I think, that got nodes in some wedges states? | 19:21 |
| jroll | yeah, we made that column unique as a result | 19:21 |
| jroll | rameshg871: looks like it kept my +2 :) | 19:22 |
| rloo | ahh. in that case, it should always be unique. so the set() isn't needed. I Don't Think.' | 19:22 |
| jroll | right | 19:22 |
| JayF | https://review.openstack.org/#/c/113865/25 has no votes jroll | 19:22 |
| jroll | looking at that right now | 19:22 |
| jroll | so many reviews | 19:23 |
| JayF | I thought that was the one you indicated kept your +2 | 19:23 |
| jroll | no, the ilo/agent did | 19:23 |
| * JayF not feeling well having trouble keeping up | 19:23 | |
| jroll | :( | 19:23 |
| rloo | Shrews: here's your chance to review 113865 again. | 19:24 |
| *** yuanying has joined #openstack-ironic | 19:25 | |
| jroll | rameshg871: +2'd ilo/iscsi | 19:25 |
| JayF | oh nooooooooo | 19:26 |
| * JayF found something | 19:26 | |
| JayF | but it's a nit at least | 19:26 |
| jroll | lol | 19:26 |
| jroll | do it | 19:26 |
| jroll | it can be fixed! | 19:26 |
| rameshg871 | jroll: i was wondering why it kept your +2 for new patch | 19:26 |
| JayF | rameshg871: want to fix it real quick? | 19:26 |
| JayF | rameshg871: found an issue in the docstring, will post the comment right meow | 19:26 |
| jroll | rameshg871: because it was a simple rebase for that patch | 19:27 |
| rameshg871 | jroll: yeah | 19:27 |
| rameshg871 | JayF: sure .. | 19:27 |
| * jroll mashes f5 waiting for JayF | 19:27 | |
| NobodyCam | lol | 19:27 |
| JayF | https://review.openstack.org/#/c/113865/25/ironic/drivers/modules/ilo/deploy.py,cm line 80 | 19:28 |
| rameshg871 | JayF: fixing right away | 19:28 |
| JayF | woo | 19:28 |
| NobodyCam | oh wow great catch JayF | 19:28 |
| JayF | heh | 19:28 |
| NobodyCam | :-p | 19:28 |
| JayF | rloo tries to get everything in one pass | 19:29 |
| JayF | I try to get one thing on every pass | 19:29 |
| JayF | lol | 19:29 |
| JayF | not literally | 19:29 |
| rloo | one is better than nothing ;) | 19:29 |
| NobodyCam | :) | 19:29 |
| jroll | whoa, you and your crazy new view | 19:29 |
| rloo | but yeah, cuz I go crazy if I do too many (> 1) passes. | 19:29 |
| JayF | did that link you directly to the new view? | 19:29 |
| JayF | haha | 19:29 |
| jroll | JayF: yeah, ",cm" | 19:30 |
| NobodyCam | :) | 19:30 |
| * NobodyCam is old school | 19:31 | |
| openstackgerrit | Ramakrishnan G proposed a change to openstack/ironic: IloVirtualMediaIscsi deploy driver https://review.openstack.org/113865 | 19:31 |
| rameshg871 | jroll: NobodyCam: ready to go again ^^^ :) | 19:31 |
| jroll | I have to re-+2 this? really? | 19:31 |
| NobodyCam | lol ok this is it for 113865, this is the version that will land! | 19:31 |
| NobodyCam | lol | 19:31 |
| jroll | done | 19:32 |
| JayF | 113865 needs +2a | 19:32 |
| JayF | dooet | 19:32 |
| jroll | also needs jenkins :| | 19:32 |
| NobodyCam | I will as soon as mr J is done | 19:32 |
| JayF | "I'm NobodyCam, and I approved this driver" | 19:32 |
| * jroll will rebase 117499 now | 19:32 | |
| Shrews | I +2'd. NobodyCam can +A | 19:32 |
| JayF | :P | 19:32 |
| jroll | so much code today | 19:32 |
| Shrews | jroll: plz to review all my codez NOW. KTHXBAI | 19:33 |
| NobodyCam | this is crush day. we tag a release tonight | 19:33 |
| *** wanyen has joined #openstack-ironic | 19:33 | |
| JayF | what is left to land? | 19:33 |
| jroll | NobodyCam: wait | 19:33 |
| JayF | ilo-ipa | 19:33 |
| *** yuanying has quit IRC | 19:33 | |
| jroll | NobodyCam: you wanted me to rebase https://review.openstack.org/#/c/117499 | 19:33 |
| jroll | but it has a dependency | 19:33 |
| jroll | NobodyCam: so I'm going to leave that alone, I think | 19:34 |
| JayF | jroll: it probably needs to be rebased off the dep, lol | 19:34 |
| jroll | JayF: well, I don't care so much about that, because dep isn't approved or even close | 19:34 |
| jroll | afaik | 19:34 |
| rloo | it is only j3. there's another release after that. | 19:34 |
| jroll | right | 19:35 |
| jroll | and this is a bugfix | 19:35 |
| openstackgerrit | Ramakrishnan G proposed a change to openstack/ironic: IloVirtualMediaAgent deploy driver https://review.openstack.org/115885 | 19:35 |
| * jroll leaves it alone | 19:35 | |
| * jroll takes a break for lunch, bbiba | 19:37 | |
| JayF | ilo-iscsi just failed tripleo, checking on it | 19:38 |
| JayF | and the only thing in the console log is it failing to upload logs | 19:39 |
| JayF | not our fault, it seems | 19:39 |
| jroll | before I go... | 19:39 |
| jroll | this needs to get through: https://review.openstack.org/#/c/118404/ | 19:40 |
| jroll | "We will refactor to split the code into submodules. Should have the new patchset tomorrow." | 19:40 |
| jroll | and stig isn't on irc | 19:40 |
| jroll | I'm fine with landing without the split (so is lucas) | 19:40 |
| jroll | will review after lunch | 19:40 |
| jroll | (also failing tempest, yay.) | 19:41 |
| openstackgerrit | A change was merged to openstack/ironic-python-agent: Add vmedia boot support in IPA https://review.openstack.org/115275 | 19:46 |
| adam_g | has anyone noticed pep8 checks randomly failing on check_uptodate.sh ? | 19:49 |
| NobodyCam | oh happy happy joy joy cable company died | 19:49 |
| * NobodyCam is now teathered to a phone :( | 19:49 | |
| NobodyCam | adam_g: the pep8 error I've seen have all been real | 19:50 |
| NobodyCam | jroll: yep your correct ... | 19:51 |
| adam_g | NobodyCam, im curious more about the check_uptodate.sh config check (whcih runs as part of pep8) i wonder if the random python hash seed stuff may be causing false negatives there. | 19:51 |
| *** rameshg871 has quit IRC | 19:51 | |
| NobodyCam | oh that I can not speak for. | 19:52 |
| *** bmahalakshmi has quit IRC | 19:54 | |
| NobodyCam | oh nice error: http://logs.openstack.org/65/113865/26/check-tripleo/check-tripleo-ironic-undercloud-precise-nonha/2d3a128/console.html | 19:55 |
| JayF | NobodyCam: is that what I pointed out earlier? | 19:56 |
| JayF | 12:39:07 <JayF> and the only thing in the console log is it failing to upload logs | 19:56 |
| NobodyCam | oh sorry I was with out connection then :-p | 19:56 |
| NobodyCam | it was so close to landing | 19:57 |
| NobodyCam | :-p | 19:57 |
| JayF | let it land, obviously the tripleo failure is unrelated | 19:58 |
| JayF | and that doesn't vote | 19:58 |
| NobodyCam | :-p | 19:58 |
| *** chuckC has joined #openstack-ironic | 19:59 | |
| NobodyCam | lol couldn't clone the devstack-gate repo | 20:00 |
| NobodyCam | :-p | 20:00 |
| NobodyCam | ok its got a few. brb | 20:01 |
| openstackgerrit | A change was merged to openstack/ironic: Add UEFI based deployment support in Ironic https://review.openstack.org/114357 | 20:01 |
| rloo | anyone know if nova is like us, they want aself.assertEqual(expected, actual)? | 20:13 |
| NobodyCam | could just add them to https://bugs.launchpad.net/ceilometer/+bug/1277104 | 20:17 |
| NobodyCam | nova cli is there | 20:17 |
| rloo | NobodyCam: this is our driver test code - 111425. | 20:20 |
| openstackgerrit | Josh Gachnang proposed a change to openstack/ironic: Improve IPA client library https://review.openstack.org/111118 | 20:22 |
| NobodyCam | rloo: where | 20:24 |
| rloo | https://review.openstack.org/#/c/111425/19/nova/tests/virt/ironic/test_driver.py, line 98, 101 (so far) | 20:24 |
| NobodyCam | rloo: did they request that our is that just a bug/error on our part | 20:26 |
| NobodyCam | that line has not changes throught all 19 reviews | 20:26 |
| rloo | I'm reviewing that patch now and noticed it. that's why I asked. | 20:26 |
| NobodyCam | I think its just our bad | 20:26 |
| NobodyCam | yea I would tag it | 20:27 |
| rloo | yeah, lucas-dinner has a patch up to fix the issues in this patch, so I'll just fix it there. | 20:27 |
| NobodyCam | lol :) | 20:27 |
| rloo | might as well make it pretty :-) | 20:27 |
| NobodyCam | ++ dont block that patch | 20:27 |
| NobodyCam | and correct | 20:27 |
| rloo | yeah, and that! | 20:28 |
| *** yuanying has joined #openstack-ironic | 20:29 | |
| openstackgerrit | Jeremy Stanley proposed a change to openstack/python-ironicclient: Work toward Python 3.4 support and testing https://review.openstack.org/118801 | 20:30 |
| openstackgerrit | Jeremy Stanley proposed a change to stackforge/pyghmi: Work toward Python 3.4 support and testing https://review.openstack.org/118829 | 20:32 |
| jroll | adam_g: have an example? | 20:32 |
| lifeless | erm wut "py27 runtests: commands[2] | bash -c cat .testrepository/1 >>.testrepository/0 | 20:33 |
| lifeless | " | 20:33 |
| lifeless | that is 100% crack | 20:33 |
| jroll | lol | 20:33 |
| lifeless | seriously, wtf, its going to be building a huge 0 file over time and have no impact on the reporting after the first time its run | 20:34 |
| jroll | lifeless: is that in jenkins jobs? if so, wouldn't .testrepository get thrown away each time, anyway? | 20:35 |
| lifeless | its also in local jobs | 20:36 |
| jroll | welp. | 20:36 |
| jroll | reporting seems to work for ironic for me, fwiw | 20:36 |
| lifeless | do me a favour | 20:36 |
| lifeless | ls -lh .testrepository/0 | 20:37 |
| jroll | wow | 20:37 |
| jroll | 46M | 20:37 |
| lifeless | and cat .testrepository/next-stream | 20:37 |
| jroll | 135 | 20:37 |
| jroll | which seems to be correct | 20:37 |
| lifeless | ls -lh .testrepository/2 | 20:37 |
| lifeless | and 3 | 20:37 |
| jroll | 58k | 20:38 |
| jroll | 671k | 20:38 |
| lifeless | yeah | 20:38 |
| lifeless | so I'm right :) | 20:38 |
| lifeless | fixing | 20:38 |
| lifeless | next time folk, talk to me | 20:38 |
| jroll | are you thinking it's inflating? | 20:38 |
| *** yuanying has quit IRC | 20:38 | |
| lifeless | every time you run it | 20:38 |
| lifeless | it being tox | 20:38 |
| jroll | because /4 is back to 59k | 20:38 |
| lifeless | its concatenating 1 onto 0 | 20:39 |
| jroll | I mean, 0 clearly is inflating like mad | 20:39 |
| lifeless | - read tox.ini | 20:39 |
| jroll | right | 20:39 |
| jroll | I see the problem, I'm just curious why /3 is important | 20:39 |
| lifeless | baselining | 20:39 |
| lifeless | there are two testsuites being run as separate things | 20:39 |
| jroll | right, ok | 20:39 |
| lifeless | one will be bigger than the other | 20:39 |
| jroll | ah, yes | 20:39 |
| jroll | forgot there's two suites | 20:40 |
| lifeless | if either of 2 or 3 was 46M I might have been wrong | 20:40 |
| jroll | on the "next time, talk to me" thing... idk what testr docs look like, but maybe there's something to note here | 20:40 |
| jroll | (depending why this was done in the first place) | 20:40 |
| jroll | shouldn't need to ask a library author how to use the library L| | 20:40 |
| lifeless | well | 20:40 |
| jroll | s/L|/:| | 20:40 |
| lifeless | the issue is upstream discover | 20:40 |
| lifeless | the talk to me bit is more if there is a limitation in a tool, lets fix the tool rather than workaround | 20:41 |
| jroll | ok, sure :) | 20:41 |
| lifeless | and honestly, concatting two files in a stateful database .... how is that not a workaround (And in this case not even a good one) | 20:43 |
| jroll | agree | 20:43 |
| * lifeless stabs pip depends a bit harder | 20:48 | |
| lifeless | I should upgrade this dev container :) | 20:48 |
| lifeless | I got onto this from the hash ring thread | 20:49 |
| lifeless | tried to run tests and my eyes bled :) | 20:49 |
| jroll | lol | 20:49 |
| lifeless | adventures in yak shaving | 20:49 |
| jroll | lifeless: bug related to that: https://bugs.launchpad.net/ironic/+bug/1355510 | 20:50 |
| lifeless | jroll: ah no thats the update-based-on-ring thing | 20:52 |
| lifeless | jroll: the thread was saying that the ring isn't stable | 20:52 |
| jroll | lifeless: right... I would just turn that bug into "our hash ring sucks in so many ways" | 20:52 |
| lifeless | oh great | 20:52 |
| lifeless | DuplicateOptError: duplicate option: rpc_backend | 20:52 |
| rloo | jroll: oh, did you say that there's room for lots of improvement with our hash ring? | 20:54 |
| jroll | uh oh | 20:54 |
| * jroll hides | 20:54 | |
| rloo | ha ha. where were you when we needed it? | 20:55 |
| jroll | oh wait, you're just nitpicking my grammar again! | 20:55 |
| jroll | I was building an agent for you :P | 20:55 |
| rloo | and a lovely agent it is! | 20:55 |
| jroll | that's questionable, but thanks :) | 20:55 |
| rloo | I know, there's room for improvement there too :D | 20:55 |
| lifeless | any thoughts on this duplicate option thing ? | 20:56 |
| lifeless | ah, I think I found it | 20:57 |
| jroll | lifeless: check your config? | 20:57 |
| lifeless | its because I'm working on fixing this two-test-suite-thing | 20:57 |
| lifeless | we need to unregister that option from nova | 20:58 |
| NobodyCam | uggh 113865 failed python 27 | 20:59 |
| jroll | wat | 21:00 |
| lifeless | jroll: was that way to NobodyCam or me ? | 21:00 |
| jroll | to NobodyCam | 21:00 |
| NobodyCam | couldn't get some dep | 21:00 |
| jroll | idk anything about that option, and not going down that rabbit hole right now | 21:00 |
| adam_g | lifeless, that will all go away when the ironic.nova.* goes away | 21:01 |
| lifeless | adam_g: yes, but its broken now. | 21:01 |
| adam_g | lifeless, the dual test suites, cat'ing of test database, oslo errors | 21:01 |
| lifeless | adam_g: and I can't ignore the egregiously broken stuff | 21:02 |
| NobodyCam | jroll: ERROR: py27: could not install deps | 21:03 |
| NobodyCam | i'll recheck as soon as i can | 21:03 |
| jroll | silly jenkins | 21:03 |
| jroll | https://review.openstack.org/#/c/118404/3/doc/source/deploy/drivers.rst | 21:04 |
| jroll | line 58 | 21:04 |
| jroll | 2c is actually a thing? that's weird | 21:04 |
| adam_g | jroll, https://bugs.launchpad.net/oslo-incubator/+bug/1365136 was mostly wondering if its showing up anywhere outside of tempest | 21:04 |
| jroll | ah, I see | 21:04 |
| jroll | adam_g: so things just move around, I guess? | 21:05 |
| mrda | Morning Ironic! | 21:05 |
| jroll | mrda! | 21:05 |
| jroll | we have a million things for you to do! | 21:05 |
| mrda | d'oh | 21:05 |
| mrda | :) | 21:05 |
| jroll | hehehe | 21:05 |
| adam_g | lifeless, test discovery attempts to import everyting under ironic.*. ironic.nova.* is importing nova config at some point, causing the conflicts on the oslo resources | 21:05 |
| jroll | I'm kidding, ignore me | 21:05 |
| lifeless | adam_g: I know, I'm fixing. | 21:05 |
| NobodyCam | morning mrda | 21:06 |
| jroll | I'd rather just land the nova driver | 21:06 |
| mrda | jroll: +1 | 21:06 |
| jroll | but... working software is cool too | 21:06 |
| NobodyCam | mrda: will we need a ffe for nova driver? | 21:06 |
| jroll | maybe if we all pounce nova-core at the same time, we won't | 21:06 |
| jroll | NobodyCam: is devanand1 back tomorrow, I guess? | 21:07 |
| NobodyCam | jroll: maybe, I supect he is looking for pants | 21:07 |
| NobodyCam | lol | 21:07 |
| NobodyCam | i'll be at nova meeting tomorrow | 21:08 |
| jroll | lol | 21:08 |
| jroll | you're supposed to burn the giant man, not your pants | 21:08 |
| NobodyCam | lol | 21:09 |
| NobodyCam | ohh thouse just get lost from what I hear | 21:09 |
| jroll | heh | 21:09 |
| jroll | always keep extra | 21:09 |
| jroll | it's like towels | 21:09 |
| jroll | never leave without one | 21:09 |
| jroll | always have spares | 21:10 |
| jroll | etcetc | 21:10 |
| NobodyCam | +42 | 21:10 |
| jroll | :D | 21:10 |
| NobodyCam | lol all out errors should start with "Don't worry" | 21:10 |
| NobodyCam | s/out/our/ | 21:11 |
| jroll | lol | 21:11 |
| jroll | I have so many minor nits I'm trying to hold back | 21:11 |
| jroll | on this snmp driver | 21:11 |
| jroll | too much code | 21:11 |
| mrda | so FYI, I believe we'll need to get a Feature Freeze Exception (FFE) for the Ironic driver from Nova now. Because not all the code has +2. mikal gets back from the USA today, and I'll approach him about it once he lands. | 21:14 |
| rloo | jroll: if there are that many, snmp isn't ready to go | 21:14 |
| rloo | adam_g: question for you about a change you made: https://github.com/openstack/ironic/commit/4d4aeab4e57d623ca06bdfdceddbee2791851d03#diff-9d1bcbeec6093089dc050d1d913a601e | 21:15 |
| mrda | unless it all lands today :-/ | 21:15 |
| NobodyCam | mrda: want to add us to the ffe section of: https://wiki.openstack.org/wiki/Meetings/Nova#Agenda_for_next_meeting | 21:15 |
| rloo | adam_g: do you recall why you used list(set(...)) in list_instance_uuids()? | 21:15 |
| mrda | yup | 21:15 |
| jroll | rloo: I'm thinking like 2 spaces instead of one in a comment | 21:15 |
| jroll | err, in a string | 21:16 |
| rloo | mrda, you're going to hate me. I -1'd one of the driver patches. | 21:16 |
| mrda | I'll rustle up some cores | 21:16 |
| mrda | rloo: saw that :) | 21:16 |
| rloo | jroll: does it happen a lot? is that the only thing? | 21:16 |
| mrda | rloo: thank you for your review though | 21:16 |
| jroll | rloo: just something I noticed | 21:16 |
| mrda | it's ok | 21:16 |
| mrda | everybody hates me :) | 21:16 |
| rloo | mrda: lucas-dinner started a patch to address them: https://review.openstack.org/#/c/118693/ | 21:16 |
| rloo | mrda: and he wanted me to update it. | 21:17 |
| * mrda needs to rustle up nova cores right now | 21:17 | |
| rloo | mrda: I, for one, am glad you are doing what you are doing ;) | 21:17 |
| mrda | rloo: thanks, just gald I can help | 21:17 |
| mrda | s/gald/glad/ | 21:17 |
| rloo | mrda: so I think we need to put up another revision for patch 111425 before asking the nova cores to look (again) | 21:18 |
| adam_g | rloo, one sec | 21:18 |
| mrda | rloo: extra_specs | 21:18 |
| mrda | rloo: not needed anymore? | 21:18 |
| rloo | mrda: lucas-dinner doesn't think so and I don't either but I don't know much. he took it out so I'm guessing we don't. | 21:18 |
| rloo | mrda: I can't figure out why in the nova patch, the LOG string (line 92 of driver.py) doesn't have _LW(), yet we have it in our ironic tree. | 21:19 |
| jroll | rloo: are we sure nova's convention is to use _LW? | 21:19 |
| jroll | (etc) | 21:20 |
| jroll | I'd be willing to bet they aren't conforming to that yet, or something | 21:20 |
| rloo | jroll: there are other _LW() in that file. we should, at the very least, be consistent. and it is _LW() in our version of it. | 21:20 |
| jroll | oh | 21:20 |
| jroll | ok | 21:20 |
| jroll | should fix thart | 21:20 |
| jroll | -r | 21:20 |
| rloo | I'm not sure how/what to update for _validate_instance_and_node() though. | 21:20 |
| rloo | the docstring is incorrect based on the code. | 21:21 |
| adam_g | rloo, it looks like, at the time, there was no unique constraint on instance_uuid in the database | 21:21 |
| mrda | Nova use _LW | 21:21 |
| rloo | adam_g: ok, that's what I thought. So I can change it now -- remove the set() part, right? | 21:21 |
| jroll | rloo: I wonder if the docstring should read 'ensure this instance is still associated with a node' | 21:22 |
| adam_g | rloo, see lucas-dinner's comments @ https://review.openstack.org/#/c/98268/1/ironic/nova/virt/ironic/driver.py | 21:22 |
| jroll | rloo: but I may be wrong | 21:22 |
| mrda | The code was changing in this area during proposal, so it's entirely possible that a few were missed. | 21:22 |
| NobodyCam | will a recheck restarjobs that are already running? | 21:22 |
| jroll | NobodyCam: idk | 21:22 |
| rloo | adam_g: thx, that comment from lucas-dinner helped. So yes, it can be removed now. | 21:22 |
| adam_g | rloo, yeah, it should be okay to use just a list straight from node.list | 21:23 |
| mrda | So it's getting serious. It has to land by tomorrow week. There's no exception beyond Friday week. So I would be arguing only necessary changes | 21:23 |
| lifeless | I thought we switched to oslo.messaging | 21:23 |
| lifeless | so why do we still have the rpc module | 21:23 |
| adam_g | rloo, im not sure thats worth the churn at this point tho? unless its noted as a blocker by nova reviews? | 21:23 |
| rloo | jroll: yeah, I think that docstring is good. My concern is deleting the stuff that is there now, although we can get it back later. | 21:23 |
| lifeless | ah... | 21:23 |
| lifeless | git fail - pyc files | 21:24 |
| rloo | jroll: I am wondering whether there was meant to be some code that checks node.uuid == instance[node] or something like that. | 21:24 |
| NobodyCam | looks like nope :( | 21:24 |
| jroll | rloo: not sure what you mean? | 21:24 |
| jroll | rloo: oh, right | 21:24 |
| jroll | rloo: not sure | 21:24 |
| rloo | adam_g: I noticed it so I should have shut up. but the code there and list_instances is inconsistent -- which is what made me wonder why they were different. | 21:24 |
| adam_g | rloo, :) | 21:25 |
| rloo | we have to update that patch anyway, so might as well do a good job on it now (as long as we don't break anything) | 21:25 |
| rloo | jroll: exactly, not sure, and adding that code *could* break something. which is why changing the docstring is safer at this point. | 21:26 |
| jroll | rloo: on the other hand, the more changes we have in there, the harder it is for nova to review | 21:26 |
| rloo | jroll: you mean if they do a diff between revisions? | 21:26 |
| lifeless | wow, ironics test suite writes to stderr a /tonne/ | 21:26 |
| jroll | yes | 21:26 |
| lifeless | result = function(*args, **kwargs) | 21:26 |
| lifeless | TypeError: <lambda>() takes no arguments (1 given) | 21:26 |
| lifeless | etc | 21:26 |
| jroll | rloo: and they likely will, who would read all the code again? | 21:27 |
| jroll | lifeless: that looks broken, I don't get that in stdout | 21:27 |
| mrda | So we really do want minimal changes to the driver | 21:27 |
| rloo | jroll: true. now you tell me. I wouldn't have put my comments in there then. | 21:27 |
| jroll | mrda: agree | 21:27 |
| mrda | only if something is broken | 21:27 |
| lifeless | jroll: its being eaten by the stdout eater, but if thats not enabled... | 21:27 |
| lifeless | jroll: try this | 21:27 |
| lifeless | OS_STDOUT_CAPTURE=0 OS_STDERR_CAPTURE=0 testr run | 21:28 |
| jroll | rloo: sigh | 21:28 |
| *** dhellmann is now known as dhellmann_ | 21:28 | |
| mrda | For example, the ironicclient caching has come up in review a few times. Nova things it's an easy fix, but they are happy to bump that to a "must fix in K". So, please, let's focus on critical for J release bugs | 21:28 |
| jroll | lifeless: /me runs | 21:28 |
| mrda | Is https://review.openstack.org/#/c/118693 critical? | 21:28 |
| lifeless | oh, sorry .testr.conf is buggy there | 21:28 |
| lifeless | let me paste a fix | 21:28 |
| mrda | (for J release in the ironic driver) | 21:28 |
| lifeless | jroll: in .testr.conf change | 21:29 |
| jroll | mrda: I think there was a -1 for that | 21:29 |
| lifeless | OS_STDOUT_CAPTURE=1 to OS_STDOUT_CAPTURE=${OS_STDOUT_CAPTURE:-1} | 21:29 |
| lifeless | jroll: and similar for ERR, then do what I said above | 21:29 |
| rloo | mrda: joe -1'd the extra-specs thing. | 21:29 |
| mrda | rloo: cool, thanks for pointing out jogo's -1 | 21:30 |
| jroll | lifeless: oh, well, I think that's e.g. LOG.exception | 21:31 |
| rloo | mrda: any way for me to undo my comments in 111425? I can remove my -1 from that patch if you want. | 21:31 |
| mrda | In that case, can we get eyeballs on https://review.openstack.org/#/c/118693 ? | 21:31 |
| mrda | I will get the change up today | 21:31 |
| jroll | lifeless: also eventlet being a piece of something | 21:31 |
| mrda | but I need 2x+2's for that :) | 21:31 |
| *** linggao has quit IRC | 21:31 | |
| jroll | mrda: just have you 1 | 21:31 |
| jroll | s/have/gave | 21:31 |
| openstackgerrit | lifeless proposed a change to openstack/ironic: Unbreak debugging via testr https://review.openstack.org/118896 | 21:31 |
| lifeless | jroll: indicates we haven't setup logging properly in the tests, at minimum :) | 21:32 |
| jroll | rloo: too late now, comments are forever | 21:32 |
| jroll | lifeless: indeed | 21:32 |
| mrda | thanks jroll | 21:32 |
| *** krtaylor has quit IRC | 21:32 | |
| rloo | jroll, mrda: so what should I do if anything? address the comments in 118693 or not? | 21:32 |
| jroll | rloo: let's not do that unless they are absolutely necessary | 21:33 |
| mrda | what comments in 118693? | 21:33 |
| rloo | jroll: so who decides which are necessary? | 21:33 |
| jroll | rloo: is something horribly broken? | 21:34 |
| mrda | oh, the review comments in 111425 that aren't yet in 118693? | 21:34 |
| rloo | mrda: my comments in 111425, that 118693 was meant to address. | 21:34 |
| mrda | right | 21:34 |
| rloo | jroll: no nothing horribly broken. | 21:34 |
| jroll | right... I would leave it | 21:34 |
| mrda | so the docstrings, go ahead and address | 21:34 |
| jroll | although I think nova might pick up on those and ask for the same :| | 21:34 |
| rloo | jroll: but the comments are in 111425. will other reviewers ask why they weren't addressed? | 21:34 |
| mrda | rloo: yes | 21:34 |
| mrda | they will ask | 21:34 |
| jroll | right | 21:34 |
| jroll | sadface | 21:35 |
| *** yuanying has joined #openstack-ironic | 21:35 | |
| * rloo has sadder face | 21:35 | |
| mrda | so I wouldn't touch the log levels | 21:35 |
| mrda | we can live with that | 21:35 |
| rloo | mrda: maybe that's why the log level is diff in ironic tree than nova patch. | 21:35 |
| mrda | we will need to handle InvalidArchitectureName | 21:35 |
| mrda | rloo: I think that's my pebcak actually | 21:36 |
| rloo | mrda: yeah. Does anyone know how? | 21:36 |
| rloo | mrda: what is a pebcak? | 21:36 |
| jroll | I had it handled once upon a time | 21:36 |
| jroll | and people hated it | 21:36 |
| jroll | yay. | 21:36 |
| rloo | mrda: I'm thinking payback, or pebble-cake. | 21:36 |
| jroll | rloo: Problem Exists Between Chair And Keyboard | 21:36 |
| mrda | problem exists between computer and keyboard | 21:36 |
| mrda | close :) | 21:36 |
| jroll | heh | 21:36 |
| rloo | ha ha | 21:36 |
| jroll | I mean... the bluetooth between my keyboard and computer isn't always happy, either | 21:37 |
| mrda | ok, so rloo would you be able to update 118693 addressing the nits and the exception? | 21:37 |
| mrda | but leaving logging alone? | 21:37 |
| rloo | ok, if InvalidArchitectureName, I'm going to set the cpu_arch to None. | 21:37 |
| rloo | mrda: yeah, there was only one logging nit anyway I think. | 21:37 |
| mrda | rloo: I think that's fair, because that's what canonicalize() does internally for None provided | 21:38 |
| mrda | and then if we can get eyeballs on that, I can get it merged into Nova this morning, and off we go. | 21:38 |
| NobodyCam | gah the queue is so slow! | 21:39 |
| mrda | Only 8 calendar days until drop dead :( | 21:39 |
| mrda | It's that time of the year NobodyCam | 21:39 |
| NobodyCam | yep | 21:39 |
| mrda | More sad is that I only have one Nova core signed up | 21:39 |
| jroll | I didn't expect it to be jogo | 21:40 |
| mrda | Strike that, two nova cores | 21:40 |
| mrda | One more to go | 21:40 |
| mrda | and dansmith | 21:40 |
| jroll | niiiice | 21:40 |
| NobodyCam | mrda: we are here if you us! | 21:40 |
| NobodyCam | gah | 21:41 |
| mrda | just get 118693 readied :P | 21:41 |
| NobodyCam | if you *NEED* us.. :-p | 21:41 |
| mrda | Nova meeting page updated | 21:43 |
| rloo | mrda: fyi, there's a -1 by joe wrt CONF.ironic.client_log_level. | 21:43 |
| *** yuanying has quit IRC | 21:43 | |
| NobodyCam | :-p | 21:44 |
| mrda | rloo: looks like a nit to me | 21:44 |
| mrda | Joe comments "yeah, I think its fine to leave as is. I would like to see a low priority bug opened against this though" | 21:45 |
| rloo | mrda: sorry, that's for the use of 'icli'. | 21:46 |
| rloo | mrda: see line 139 | 21:46 |
| rloo | mrda: that reminds me, are you opening bugs when they ask for them? | 21:46 |
| mrda | rloo: I will, but haven't. Because that only started with Joe overnight | 21:47 |
| mrda | I can do that | 21:47 |
| rloo | thx mrda. didn't know that was a new thing. | 21:47 |
| mrda | rloo: but you're right re: Joe's logging conf | 21:48 |
| mrda | It appears he wants this changed | 21:48 |
| mrda | rloo: can you do that? | 21:49 |
| rloo | mrda: our version of driver.py doesn't have the arch.canonicalize() stuff? | 21:49 |
| rloo | mrda: do what? | 21:49 |
| mrda | rloo: logging changes | 21:49 |
| rloo | mrda: I'm NOT undoing Chris behren's changes. | 21:49 |
| jroll | he does want it changed? :/ | 21:49 |
| rloo | jroll: Joe thinks we can set the ironicclient logging via nova's logging.conf stuff. | 21:50 |
| mrda | rloo: there is a review for the canonicalize change, but I haven't updated it yet. We did this one Nova-first-then-ironic since we were finding it hard to get agreement on the way forward | 21:50 |
| rloo | jroll: I have no idea how, but it seems to me that Chris would have known if there was an 'easy' way? | 21:50 |
| mrda | so I'll backport that change into ironic | 21:50 |
| jroll | comstud: ^^ | 21:51 |
| rloo | mrda: should I wait for the backport, to get it into 118693? | 21:51 |
| jroll | rloo: if that's doable, then let's do it, but that's a nit imo | 21:51 |
| jroll | well, maybe not, config options are important | 21:51 |
| rloo | comstud: more specifically, see line 139: https://review.openstack.org/#/c/111425/19/nova/virt/ironic/driver.py | 21:52 |
| mrda | Just do it independently 116772 is the patch for canonicalize | 21:52 |
| mrda | oh I see | 21:52 |
| mrda | Ok, should we rebase 118693 on top of 116772? | 21:52 |
| rloo | mrda: that's fine. lucas' patch can be rebased after 116772 lands. | 21:53 |
| mrda | rloo: does that work for you? (if I update 116772 immediately) | 21:53 |
| rloo | mrda: or we could rebase on top of... | 21:53 |
| mrda | I'll update that in 5 minutes (need to grab breakfast first ;) | 21:53 |
| jroll | let me know if I can help at all, bt | 21:54 |
| jroll | w | 21:54 |
| comstud | hm | 21:58 |
| mrda | jroll: can you look at comstud's comment on L124 of https://review.openstack.org/#/c/116772/5/ironic/nova/virt/ironic/driver.py to see if we need to do anything? | 21:58 |
| mrda | comstud: oh hai | 21:58 |
| jroll | mrda: he took that back in irc but not in gerrit | 21:59 |
| mrda | ahh | 21:59 |
| jroll | mrda: the code is fine | 21:59 |
| mrda | \o/ | 21:59 |
| * mrda stops arm flailing | 21:59 | |
| comstud | mrda: my comment is wrong | 22:00 |
| comstud | sorry, i should have replied there | 22:00 |
| comstud | but i replied in the main review comment area | 22:00 |
| * comstud adds a reply there | 22:01 | |
| comstud | but from my same reply earlier | 22:02 |
| comstud | "So, I guess I also have concerns that this may potentially make the image_props_filter not match anymore for certain people's configs..." | 22:02 |
| mrda | right | 22:02 |
| openstackgerrit | Ruby Loo proposed a change to openstack/ironic: Driver merge review comments from 111425 https://review.openstack.org/118693 | 22:04 |
| openstackgerrit | Kyle Stevenson proposed a change to openstack/ironic: Improve IPA client library https://review.openstack.org/111118 | 22:04 |
| kylestev | JoshNang: ^ | 22:04 |
| rloo | mrda, jroll: I pushed up another revision. it doesn't have the fix to address the arch exception (needs to be rebased first), and joe has some comments in test_driver.py that aren't addressed. | 22:05 |
| jroll | rloo: what comments aren't addressed? (and why?) | 22:06 |
| rloo | mrda, jroll: the dinner bell has rung so I need to make an appearance. I'll check in about an hour or so. | 22:06 |
| jroll | rloo: also, can you go ahead and rebase? | 22:06 |
| jroll | oh | 22:06 |
| jroll | ok | 22:06 |
| rloo | jroll: he wanted to combine two tests some how. | 22:06 |
| mrda | thanks rloo ! | 22:06 |
| jroll | huh. | 22:06 |
| jroll | rloo: so it definitely needs work? | 22:06 |
| mrda | I'll have 116772 updated by then | 22:06 |
| jroll | I can try to finish it | 22:06 |
| rloo | https://review.openstack.org/#/c/111425/19/nova/tests/virt/ironic/test_driver.py | 22:06 |
| jroll | once mrda is done mucking aroudn | 22:06 |
| mrda | lol! | 22:06 |
| rloo | oh, there were two comments joe had, see the bottom of the page. | 22:06 |
| NobodyCam | rloo: Get a node associated with an instance. that implys that a instance can have more then one node? would not Get node associated with instance. be more correct? | 22:06 |
| rloo | NobodyCam: 'Get the node associated with the instance'? Sorry, I was in a hurry. jroll will update it, won't you jroll? Thx! | 22:07 |
| jroll | suuuuuuuuuuurrrrrrrrrrrre | 22:07 |
| mrda | rloo: go eat dinner! | 22:08 |
| jroll | whatever you say | 22:08 |
| openstackgerrit | Kyle Stevenson proposed a change to openstack/ironic: Improve IPA client library https://review.openstack.org/111118 | 22:08 |
| NobodyCam | lol | 22:08 |
| NobodyCam | enjoy your dinner rloo :) | 22:09 |
| *** openstack has joined #openstack-ironic | 22:11 | |
| NobodyCam | jroll: your reworking 118693? | 22:13 |
| jroll | NobodyCam: not yet, feel free to take it | 22:13 |
| jroll | I'm looking at snmp | 22:13 |
| jroll | still | 22:14 |
| jroll | trying not to get distracted :| | 22:14 |
| mrda | NobodyCam: I'm just testing 116772 which should probably be a dep for 118693 | 22:15 |
| NobodyCam | mrda: I have time to go somke before you push up 116772? | 22:24 |
| mrda | ya | 22:24 |
| NobodyCam | :) brb | 22:24 |
| NobodyCam | :-p | 22:24 |
| jroll | last call for -1's on https://review.openstack.org/#/c/118404/ | 22:25 |
| jroll | I'm ready to land it | 22:25 |
| jroll | stepping away for a few real quick | 22:25 |
| jroll | NobodyCam etc ^^ | 22:25 |
| jroll | actually, I'm just going to land it, y'all have from now until gate jobs pass to strike it down :) | 22:25 |
| mrda | jroll: I used to work at a company that launched a product without a way to bill for it, knowing that they had a month to write the billing interface, before the 1st customer needed to be billed. | 22:27 |
| mrda | It's a bit like that :) | 22:27 |
| jroll | heh | 22:27 |
| openstackgerrit | Syed Ismail Faizan Barmawer proposed a change to openstack/ironic: Add uefi boot mode support in IloVirtualMediaIscsiDeploy https://review.openstack.org/116561 | 22:28 |
| * mrda thinks we need to parallelise pep8 | 22:30 | |
| kylestev | mrda: +1... it's really slow :( | 22:32 |
| NobodyCam | gah tempest test still over an hour away | 22:33 |
| NobodyCam | for 113865 | 22:33 |
| kylestev | mrda: alternatively, you could use an OnMetal IO instance :P | 22:37 |
| mrda | kylestev: +1 | 22:37 |
| mrda | I'm actually using a 30Gb Perf2 | 22:37 |
| kylestev | oooh, nice choice! | 22:37 |
| jroll | perf cloud ftw | 22:37 |
| jroll | well | 22:38 |
| mrda | but pep8 is still serial | 22:38 |
| jroll | oops, I'm not supposed to say that out loud :P | 22:38 |
| kylestev | ha | 22:38 |
| * jroll brb | 22:38 | |
| kylestev | being a core must so sooo inconvenient | 22:38 |
| *** krtaylor has joined #openstack-ironic | 22:39 | |
| *** yuanying has joined #openstack-ironic | 22:40 | |
| mrda | kylestev: but at least my py27 runs are quick | 22:41 |
| openstackgerrit | Michael Davies proposed a change to openstack/ironic: Nova review updates for _node_resource https://review.openstack.org/116772 | 22:41 |
| mrda | NobodyCam: ^^ feel free to rebase on top of this | 22:42 |
| NobodyCam | mrda: | 22:42 |
| NobodyCam | mrda: ack | 22:42 |
| lifeless | success | 22:42 |
| NobodyCam | :) | 22:42 |
| lifeless | Ran 1365 tests in 67.274s | 22:42 |
| lifeless | OK | 22:42 |
| lifeless | jroll: adam_g:^ | 22:42 |
| NobodyCam | lifeless: :) | 22:43 |
| lifeless | one run | 22:44 |
| adam_g | lifeless, cool. what'd you need change? | 22:46 |
| *** yuanying has quit IRC | 22:48 | |
| lifeless | adam_g: incoming | 22:51 |
| openstackgerrit | lifeless proposed a change to openstack/ironic: Run the whole test suite as one run https://review.openstack.org/118920 | 22:51 |
| jroll | kylestev: I laughed, but still not sure what you mean :P | 22:53 |
| lifeless | mrda: pep8 is parallelised in the next release | 22:54 |
| lifeless | mrda: though why it needs it is terrible :) | 22:54 |
| NobodyCam | mrda: is this right? http://paste.openstack.org/show/8u4fMymo4sHXBPLoi5cv/ | 22:55 |
| mrda | lifeless: did we discuss this last week? | 22:55 |
| lifeless | mrda: it was discussed in #openstack-infra the other day | 22:55 |
| mrda | lifeless: ahh ok | 22:55 |
| openstackgerrit | lifeless proposed a change to openstack/ironic: Ignore backup files https://review.openstack.org/118922 | 22:56 |
| mrda | NobodyCam: wow | 22:56 |
| mrda | Now I'm not so sure | 22:56 |
| NobodyCam | ya | 22:56 |
| NobodyCam | I'm going to not do that | 22:56 |
| mrda | good plan | 22:56 |
| NobodyCam | :-p | 22:56 |
| mrda | :) | 22:56 |
| kylestev | jroll: not being able to talk up your employer's products | 22:57 |
| mrda | Ok, I'll bbs. School run time. | 22:57 |
| jroll | kylestev: nah, as an onmetal dev I'm supposed to say "virtualization is dead" | 22:58 |
| jroll | or something | 22:58 |
| kylestev | ahhh | 23:00 |
| lifeless | adam_g: thoughts appreciated on that patch; if its too ugly to bear I can just use it locally to let me be sane in the interim | 23:00 |
| jroll | kylestev: (long live virtualization) :P | 23:01 |
| openstackgerrit | Chris Krelle proposed a change to openstack/ironic: Driver merge review comments from 111425 https://review.openstack.org/118693 | 23:01 |
| jroll | did we actually land all the j3 blueprints? | 23:01 |
| jroll | I think we did | 23:01 |
| jroll | holy cow | 23:01 |
| NobodyCam | if we can them thru the gate | 23:02 |
| jroll | right | 23:02 |
| jroll | s/land/approve/ | 23:02 |
| jroll | :) | 23:02 |
| NobodyCam | :) | 23:03 |
| * NobodyCam checks zuul and goes out for a smoke | 23:03 | |
| jroll | NobodyCam: were you going to rebase that on top of the other thing or? | 23:03 |
| NobodyCam | jroll: no ... 4 RE: see ttp://paste.openstack.org/show/8u4fMymo4sHXBPLoi5cv/ | 23:04 |
| * jroll tries to find the other one | 23:04 | |
| jroll | right, I saw that | 23:04 |
| NobodyCam | :-p | 23:04 |
| jroll | I can try to rebase if you'd like | 23:04 |
| NobodyCam | you welcome to :) | 23:04 |
| jroll | I don't think it matters, though | 23:05 |
| NobodyCam | I just review -d 116772 and then git rebase review/michael_davies/116772 | 23:05 |
| jroll | let's just land both | 23:05 |
| jroll | oh | 23:05 |
| jroll | I usually do review -X 116772 | 23:05 |
| jroll | it's fine | 23:05 |
| jroll | they don't need to depend, just both need to land | 23:05 |
| * NobodyCam points angry finger ar mr J | 23:06 | |
| NobodyCam | s/ar/at/ | 23:06 |
| wanyen | jroll: ilo/uefi boot still need review | 23:06 |
| wanyen | jroll, ilo/ipa and ilo/iscsi driver have not yet merged. right? | 23:07 |
| jroll | wanyen: was ilo in the uefi spec? | 23:07 |
| jroll | wanyen: they have not merged, they have been approved, I think | 23:07 |
| * jroll double checks | 23:07 | |
| NobodyCam | ilo/iscsi is in gate now... will need recheck | 23:08 |
| NobodyCam | but it trying to land | 23:08 |
| jroll | I don't see uefi for ilo in a spec | 23:08 |
| wanyen | jroll: uefi spec cover both pxe and vendor drivers | 23:08 |
| * jroll looks | 23:08 | |
| wanyen | jroll: Add uefi boot mode support in IloVirtualMediaIscsiDeploy https://review.openstack.org/116561 | 23:09 |
| jroll | thanks | 23:09 |
| jroll | that's a lot of code | 23:09 |
| jroll | and failing tests | 23:09 |
| jroll | and we have... 51 minutes by my clock | 23:09 |
| jroll | (my clock may be off by a few hours | 23:09 |
| jroll | but there's only two cores around by now, as far as I can tell | 23:09 |
| openstackgerrit | Kyle Stevenson proposed a change to openstack/ironic: Improve IPA client library https://review.openstack.org/111118 | 23:10 |
| jroll | this wasn't on the priority list for j3; we can try to push it through but I can't make a promise | 23:10 |
| wanyen | jroll: I thought the last patch was fine | 23:10 |
| jroll | oh, jenkins is still running | 23:10 |
| jroll | wanyen: I'm still not going to promise that this will land | 23:11 |
| mrda | . | 23:12 |
| wanyen | jroll: whateve you can help will be highly appreciated. Doe silo/ipa get approved. Last time I check it only got one +2 | 23:12 |
| jroll | wanyen: oops, we'll need to bug NobodyCam to land it | 23:13 |
| wanyen | jroll: the uefi boot was on teh J3 list and the blue print and design spec cover both pxe and vendor drivers | 23:13 |
| adam_g | lifeless, added one note, but i'm okay with adding it for now | 23:14 |
| jroll | wanyen: again, we will try. but working code was not uploaded until 22:28 on feature freeze day, and we still don't know if it's working | 23:15 |
| jroll | wanyen: so you understand where I'm coming from | 23:15 |
| jroll | wanyen: have you reviewed the code? | 23:15 |
| wanyen | jroll, yes. I understand. | 23:16 |
| jroll | wanyen: all the reviews we can get will help | 23:16 |
| * mrda just reviewed 118693. Others who care about the Ironic driver landing in Nova should likewise. | 23:16 | |
| mrda | :-) | 23:16 |
| jroll | so many distractions today | 23:17 |
| jroll | "Returns True if unacceptable." | 23:17 |
| jroll | should this be :returns: True if unacceptable? | 23:18 |
| jroll | I don't actually care much | 23:18 |
| * NobodyCam thinks 113865 will the que for at least 10 hours | 23:18 | |
| NobodyCam | I saw that too | 23:18 |
| NobodyCam | but nothing else in that file follows that | 23:18 |
| jroll | great | 23:19 |
| jroll | I would assume that would give Ruby a heart attack | 23:19 |
| jroll | imbw | 23:19 |
| jroll | anyway, +2'd | 23:19 |
| *** openstackstatus has quit IRC | 23:19 | |
| jroll | NobodyCam: if the other ironic cores are gone for the day, I would say we should just approve that | 23:19 |
| * jroll is probably a bad person for saying that | 23:19 | |
| *** openstackstatus has joined #openstack-ironic | 23:20 | |
| *** ChanServ sets mode: +v openstackstatus | 23:20 | |
| NobodyCam | no it needs to land | 23:20 |
| jroll | ok | 23:20 |
| jroll | not sure if rloo is coming back | 23:20 |
| jroll | although she also pushed a patch | 23:20 |
| NobodyCam | but going to be in jenkins hands for a while | 23:20 |
| jroll | want me to just land it? | 23:20 |
| *** yuanying has joined #openstack-ironic | 23:20 | |
| jroll | oh, right, jenkins | 23:21 |
| mrda | jroll: rloo said she'd be back in an hour | 23:21 |
| jroll | poor zuul | 23:21 |
| mrda | after dinner | 23:21 |
| jroll | mrda: right, but I realized she also touched that | 23:21 |
| mrda | y'all should just work in my timezone | 23:21 |
| mrda | it would everything *so* much easier | 23:21 |
| mrda | :) | 23:21 |
| jroll | we should all just work on utc :P | 23:21 |
| jroll | so, when is j3 cut? end of day utc? | 23:22 |
| wanyen | jroll, tx. ilo-iscsi uefi boot will remove teh need for manual setting of boot device and boot mode so it will really help user's uefi boot expereicne. | 23:22 |
| NobodyCam | jroll: by that its 11 pm and I should be in bed | 23:22 |
| jroll | wanyen: (for HP hardware) | 23:22 |
| wanyen | jroll, yes. | 23:22 |
| jroll | NobodyCam: by that it's 11pm and we still have hours of sunlight to party! | 23:22 |
| mrda | NobodyCam: ouch. 9am-ish here | 23:22 |
| jroll | wanyen: I udnerstand the benefits, not everyone has hp hardware :( | 23:23 |
| wanyen | jroll, is it possible to apply for exception in case it does not land in time for J3. | 23:23 |
| NobodyCam | We should be able to recheck running jobs | 23:23 |
| * NobodyCam pops over to infra | 23:24 | |
| jroll | wanyen: I think only devanand1 can approve exceptions | 23:24 |
| jroll | wanyen: like I said, we want this code, we will try to get it through | 23:24 |
| jroll | wanyen: I just am not going to make a promise | 23:24 |
| wanyen | jroll, I see. Your andotehr reviewes help will be much appreciated. | 23:25 |
| *** lucas-dinner has quit IRC | 23:25 | |
| wanyen | jroll, tx. | 23:25 |
| openstackgerrit | A change was merged to openstack/ironic: Adds SNMP power driver https://review.openstack.org/118404 | 23:26 |
| jroll | woo ^ | 23:26 |
| *** coolsvap has quit IRC | 23:29 | |
| mrda | NobodyCam: just wondering if you could sanity check 118693 | 23:30 |
| mrda | but if it's too late, then I understand | 23:30 |
| *** chuckC has quit IRC | 23:30 | |
| * mrda is a morning person | 23:30 | |
| NobodyCam | mrda: where? | 23:30 |
| mrda | NobodyCam: https://review.openstack.org/#/c/118693/3 | 23:30 |
| wanyen | NobodyCam: can you review and approve ilo-ipa driver https://review.openstack.org/#/c/115885/ ? | 23:30 |
| mrda | forget me, you uploaded it :) | 23:31 |
| * mrda needs to wait for rloo to return | 23:31 | |
| NobodyCam | huh | 23:31 |
| jroll | mrda, rloo also touched that patch | 23:32 |
| mrda | lol | 23:32 |
| jroll | mrda: I'm ok with approving it myself, without another reviewer | 23:32 |
| jroll | special circumstances (tm) | 23:32 |
| jroll | mrda: but need jenkins first :| | 23:32 |
| mrda | it's just with the check queue so long... | 23:32 |
| jroll | and then have to wait hours | 23:32 |
| jroll | yeah | 23:32 |
| jroll | I mean, I can push the button, I don't care much for our non-voting jobs | 23:33 |
| NobodyCam | we can try and change 113865's commit message that will re queue it | 23:33 |
| jroll | (mostly joking) | 23:33 |
| mrda | And once I've got check and a +A I need to get it through Nova's check | 23:33 |
| jroll | NobodyCam: go for it | 23:33 |
| mrda | before nove cores look at it | 23:33 |
| mrda | le sigh | 23:33 |
| * mrda is preparing for Paris | 23:33 | |
| jroll | mrda: you don't *have* to wait for an ironic +A to put it in the nova patch | 23:33 |
| mrda | well, Deva would beat me up if you guys didn't approave anything non-trivial and I proposed it to Nova | 23:34 |
| mrda | you'd run me out of town! | 23:34 |
| *** coolsvap has joined #openstack-ironic | 23:34 | |
| jroll | gimme a break | 23:34 |
| jroll | it's gotta go | 23:35 |
| *** romcheg1 has quit IRC | 23:35 | |
| openstackgerrit | Chris Krelle proposed a change to openstack/ironic: IloVirtualMediaIscsi deploy driver https://review.openstack.org/113865 | 23:36 |
| NobodyCam | jroll: ^^^ only change was commit message in web interface | 23:36 |
| jroll | whee | 23:36 |
| * jroll doublechecks | 23:36 | |
| NobodyCam | at least this way we're not waiting for it to finish jsut to recheck it | 23:38 |
| mrda | ok, I'm going to merge 118693 back into Nova and upload a new revision of 111425 | 23:40 |
| jroll | mrda: we still need the canonicalize() change +2'd yeah? | 23:40 |
| mrda | that's back into Ironic, so less urgent | 23:41 |
| mrda | (ref https://review.openstack.org/#/c/116772/6) | 23:41 |
| jroll | oh, is that in nova already? | 23:41 |
| mrda | yeah, I proposed it over there as we're trying to get Nova consensus on it. | 23:42 |
| mrda | No point us all agreeing on a way forward and Nova rejecting it | 23:42 |
| jroll | right, ok | 23:42 |
| mrda | So I reversed the ordering on that | 23:42 |
| jroll | so that's... yeah | 23:42 |
| Shrews | gah. 113865 still not thru? | 23:42 |
| jroll | lol | 23:42 |
| jroll | NobodyCam: OH MY GOD | 23:42 |
| jroll | NobodyCam: 113865 needs a rebase now | 23:43 |
| jroll | ffs | 23:43 |
| NobodyCam | Shrews: I just checnged the commit message to retrigger the job | 23:43 |
| NobodyCam | no | 23:43 |
| mrda | Shrews: >8 hour check gate | 23:43 |
| jroll | yes | 23:43 |
| jroll | look again | 23:43 |
| Shrews | mrda: that is clearly your fault :-P | 23:43 |
| mrda | lol | 23:43 |
| mrda | I wish I had so much power | 23:43 |
| jroll | the snmp driver landed, maybe related | 23:43 |
| jroll | likely related, actually | 23:43 |
| jroll | ughhhhhhhh | 23:43 |
| jroll | NobodyCam: want me to get that? | 23:43 |
| mrda | Can we have another 1000 nodes added to nodepool? | 23:44 |
| jroll | oh wait, Shrews is here :o | 23:44 |
| jroll | mrda: talk to whoever does quota at rax | 23:44 |
| jroll | I vote yes | 23:44 |
| * Shrews is not the core you are looking for </waving hands> | 23:44 | |
| mrda | OH, SHREWS IS HERE | 23:44 |
| NobodyCam | jroll: I'm on phone bandwidth could you | 23:44 |
| Shrews | jroll: srsly though, need something? | 23:44 |
| jroll | Shrews: can you look at https://review.openstack.org/118693 | 23:44 |
| mrda | jroll: +1 | 23:44 |
| jroll | NobodyCam and rloo both touched it, so they won't +2 | 23:45 |
| jroll | NobodyCam: np | 23:45 |
| * Shrews puts beer down... looks | 23:45 | |
| mrda | and we're a bit short on cores in this TZ :) | 23:45 |
| Shrews | aye | 23:45 |
| jroll | nonono keep the beer up | 23:45 |
| openstackgerrit | Jim Rollenhagen proposed a change to openstack/ironic: IloVirtualMediaIscsi deploy driver https://review.openstack.org/113865 | 23:47 |
| jroll | NobodyCam: ^ | 23:47 |
| jroll | NobodyCam: only setup.cfg conflicted, I'm going to +2 due to that | 23:47 |
| NobodyCam | TY jroll :) | 23:47 |
| NobodyCam | ya | 23:48 |
| rloo | hi, am I back just in time not to need to review anything? | 23:48 |
| NobodyCam | this was to land at like 9:30 this morning | 23:48 |
| jroll | rloo: lol | 23:49 |
| Shrews | mrda: ironic_driver.... was this ever used? | 23:49 |
| NobodyCam | lol | 23:49 |
| NobodyCam | oh no we have things to review | 23:49 |
| Shrews | i mean, i guess it was, but i suppose it's an ancient artifact now | 23:49 |
| mrda | Shrews: no-one seems to think so. It's a baremetal vestige | 23:49 |
| jroll | so what's actually left to review.. | 23:49 |
| mrda | copied over and modified | 23:49 |
| Shrews | yeah | 23:49 |
| jroll | rloo: you should also look at https://review.openstack.org/118693 even though you touched it | 23:50 |
| rloo | looking... | 23:50 |
| NobodyCam | rloo: are you ok with my changes to https://review.openstack.org/#/c/118693 | 23:50 |
| jroll | need one more +2 on https://review.openstack.org/#/c/115885/ | 23:50 |
| rloo | so wrt 118693, we decided not to address Joe's comments in test_driver.py? | 23:50 |
| NobodyCam | jroll: it too needs rebase now | 23:51 |
| jroll | and need eyes on this, but not as urgent: https://review.openstack.org/116561 | 23:51 |
| jroll | NobodyCam: right, that | 23:51 |
| * jroll does it | 23:51 | |
| mrda | yay team! | 23:51 |
| NobodyCam | mrda: ??? | 23:51 |
| openstackgerrit | Jim Rollenhagen proposed a change to openstack/ironic: IloVirtualMediaAgent deploy driver https://review.openstack.org/115885 | 23:52 |
| jroll | NobodyCam: clean rebase ^^ | 23:52 |
| rloo | jroll: wrt 118693 and your nit in driver.py, the reason why I didn't do a ":returns:' is cuz that would mean doing a :param: and I wasn't sure if I should be making so many changes. | 23:52 |
| jroll | rloo: fair enough, thanks for the update :) | 23:52 |
| rloo | that driver.py is inconsistent wrt docstrings | 23:52 |
| jroll | yeah | 23:53 |
| rloo | so 118693 wasn't rebased on top of that other one with the arch.canonicalize() stuff. | 23:53 |
| rloo | but we need to make a change to the arch.canonicalize() stuff, unless one of you already did that. let me find/look at the other patch | 23:53 |
| jroll | right, the rebase went crazy | 23:54 |
| jroll | rloo: I think its already in nova | 23:54 |
| mrda | FWIW, I added in the exception handler for canonicalize back into ironic via 116772 | 23:55 |
| mrda | jroll: rloo: correct, the canonicalize stuff is already proposed in nova. | 23:55 |
| rloo | mrda. great, that's what i was hoping. | 23:55 |
| jroll | cool | 23:55 |
| mrda | I did that one backwards to try and get nova core signoff before we modified our tree | 23:55 |
| NobodyCam | jroll: 115885 now has two +2's | 23:55 |
| jroll | NobodyCam: neat, thanks :D | 23:55 |
| mrda | ...because everyone was indecisive on that issue | 23:56 |
| Shrews | mrda: 118693 lgtm. can be +A'd now, or do you just need the +2? | 23:56 |
| *** chuckC has joined #openstack-ironic | 23:58 | |
| Shrews | or, jroll, even | 23:58 |
| jroll | Shrews: needs jenkins | 23:58 |
| Shrews | pfft. what does he know | 23:58 |
| mrda | Shrews: Just a +2 for now, when Jenkins happens, then we can +A | 23:58 |
| NobodyCam | Shrews: do we need 116772 first? | 23:58 |
| Shrews | it's +2'd | 23:58 |
| mrda | Shrews: thanks | 23:58 |
| rloo | mrda: so there's still the issue of that CONF.ironic.client_log_level. Do you have to ask Joe if he is -1 against it? | 23:59 |
| jroll | Shrews: he's a big jerk | 23:59 |
| jroll | NobodyCam: 116772 is already in nova | 23:59 |
| mrda | rloo: I'll see if I can awaken him | 23:59 |
| Shrews | NobodyCam: ugh. haven't seen that one. | 23:59 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!