[14:23] <smoser> beantaxi: it woudlnt be a new stage
[14:27] <smoser> but the _default_handlers is how all the types of handlers are registereed
[14:29] <smoser> and yeah... the sub_handlers bit isn't clear
[14:30] <smoser> but i think you should add your types to that list.
[14:30] <smoser> git blame shows c7555762f3a30190ce7726b4d013bc3e83c7e4b6
[14:31] <smoser> indicatting that the sub_handler bit is what allows jinja rendering also. and i think we would want that.
[16:04] <beantaxi> smoser: Interesting ... from the code, the Jinja handler is actually added to def_handlers not sub_handlers. There appear to be 3 styles of adding a handler in stages.py:_default_handlers: 1. Instantiate the handler, assign to a variable, then add the variable to def_handles 2. Instantiate & assign to handler 'inline' 3. Add to sub_handlers. cloud_config & shellscript are #1 and #3, boothook and
[16:04] <beantaxi> upstart are #2, and then jinja is #1 but it comes after everything else.
[16:06] <beantaxi> Also the function is preceded by a comment that says '# TODO(harlowja) Hmmm, should we dynamically import these??' So I wonder if this is a decent time to make them a bit more consistent, possibly sort out sub_handlers, and make the whole thing dynamic if deiresd. (Let me know if I'm overstepping)
[17:19] <smoser> beantaxi: i think you understand...
[17:20] <smoser> yes.. i was wondering why all the handlers didn't get that
[17:55] <smoser> i think they problably should have
[17:59] <beantaxi> smoser: It also maybe looks like the point is to create all the handlers except Jinja with **opts ... and then to add a sub_handlers collection to **opts, add all the handlers to sub_handlers, and instantiate the Jinja handler with the new **opts that has the list of subhandlers
[18:02] <beantaxi> smoser: Perhaps this is because the Jinja handler is user to handle user-data scripts, so it needs to be a handler, and cloud config files, so it needs access to all the other handlers
[18:05] <beantaxi> iow it's a bit of a hybrid
[18:23] <smoser> beantaxi: i think you're down the right path.
[18:24] <smoser> the point i was trying to mnake in the review was that if we want people to be able to use this, then we should just add it to cloud-init "proper" like the other handlers.
[19:22] <beantaxi> smoser: That makes sense. In my own bumbling way, I was starting out by not wanting to touch any existing code if I could avoid it (which I did! go me), and to be happy to go in and modify existing code but only if recommended.
[19:42] <beantaxi> smoser: I think that's plenty for me to go on. I'll drop off irssi for now, and put any other questions/comments on my next commits. Thanks!
[21:46] <Odd_Bloke> falcojr: blackboxsw: https://github.com/canonical/cloud-init/pull/761 <-- PR for a test for today's hot issue
[22:04] <blackboxsw> Odd_Bloke: approved https://github.com/canonical/cloud-init/pull/762/files
[22:06] <blackboxsw> @Odd_Bloke will you be dputing this PR via build-and-push then? or your own mechanisms to dput and git push ubuntu/devel
[22:33] <blackboxsw> Odd_Bloke: +1 on groovy
[22:33] <blackboxsw> https://github.com/canonical/cloud-init/pull/763/files
[22:35] <blackboxsw> and good thinking including the LP bug in debian/changelog
[22:37] <Odd_Bloke> We need to for SRU process, so glad I caught that before upload!
[22:40] <blackboxsw> https://github.com/canonical/cloud-init/pull/764
[22:40] <blackboxsw> focal approved
[22:46] <blackboxsw> bionic approved https://github.com/canonical/cloud-init/pull/765#pullrequestreview-565799229
[22:53] <blackboxsw> Xenial approved https://github.com/canonical/cloud-init/pull/767#pullrequestreview-565802898
[22:55] <Odd_Bloke> Thanks; all uploaded and pushed.
[22:55] <Odd_Bloke> And I've dropped a message in #ubuntu-release to ask for them to be accepted to -proposed.
[23:45] <blackboxsw> Odd_Bloke: and AnhVoMSFT  our uploads are queued into (xenial|bionic|focal|groovy)-proposed pockets and should be in cloud-images tomorrow for our testing