=== cpaelzer__ is now known as cpaelzer [14:23] beantaxi: it woudlnt be a new stage [14:27] but the _default_handlers is how all the types of handlers are registereed [14:29] and yeah... the sub_handlers bit isn't clear [14:30] but i think you should add your types to that list. [14:30] git blame shows c7555762f3a30190ce7726b4d013bc3e83c7e4b6 [14:31] indicatting that the sub_handler bit is what allows jinja rendering also. and i think we would want that. [16:04] 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] upstart are #2, and then jinja is #1 but it comes after everything else. [16:06] 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] beantaxi: i think you understand... [17:20] yes.. i was wondering why all the handlers didn't get that === vrubiolo1 is now known as vrubiolo === ijohnson is now known as ijohnson|lunch [17:55] i think they problably should have [17:59] 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] 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] iow it's a bit of a hybrid [18:23] beantaxi: i think you're down the right path. [18:24] 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] 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] 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! === ijohnson|lunch is now known as ijohnson [21:46] falcojr: blackboxsw: https://github.com/canonical/cloud-init/pull/761 <-- PR for a test for today's hot issue [22:04] Odd_Bloke: approved https://github.com/canonical/cloud-init/pull/762/files [22:06] @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] Odd_Bloke: +1 on groovy [22:33] https://github.com/canonical/cloud-init/pull/763/files [22:35] and good thinking including the LP bug in debian/changelog [22:37] We need to for SRU process, so glad I caught that before upload! [22:40] https://github.com/canonical/cloud-init/pull/764 [22:40] focal approved [22:46] bionic approved https://github.com/canonical/cloud-init/pull/765#pullrequestreview-565799229 [22:53] Xenial approved https://github.com/canonical/cloud-init/pull/767#pullrequestreview-565802898 [22:55] Thanks; all uploaded and pushed. [22:55] And I've dropped a message in #ubuntu-release to ask for them to be accepted to -proposed. [23:45] 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