19:00:00 <nitzmahone> #startmeeting Ansible Core Public IRC Meeting
19:00:00 <zodbot> Meeting started Tue Dec  8 19:00:00 2020 UTC.
19:00:00 <zodbot> This meeting is logged and archived in a public location.
19:00:00 <zodbot> The chair is nitzmahone. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:00:00 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
19:00:00 <zodbot> The meeting name has been set to 'ansible_core_public_irc_meeting'
19:00:49 <jborean93> hey
19:00:55 * shertel waves
19:01:05 <nitzmahone> #info agenda https://github.com/ansible/community/issues/570
19:02:03 <nytelife26> hello
19:02:05 <nitzmahone> #chair jborean93
19:02:05 <zodbot> Current chairs: jborean93 nitzmahone
19:02:41 <nitzmahone> Looks like only one item on the agenda today: https://github.com/ansible/community/issues/570#issuecomment-738451857
19:02:59 <nytelife26> this is my first time attending a meeting so apologies if i get anything wrong
19:03:04 <jborean93> welcome
19:03:21 <nytelife26> and it seems that way, unless anyone has anything they'd like to discuss first? if not, i suppose i'll go right ahead
19:03:33 <nitzmahone> nytelife26: no worries, yeah, if there's some upfront explanation, go for it
19:04:41 <nytelife26> alright. as pertaining to https://github.com/ansible/ansible/issues/72829, their issue is that upon querying hostvars from a playbook, if the var is a template that doesn't lead anywhere it passes silently. they'd like to have a warning or an option to error for that, it seems.
19:05:52 <nytelife26> i'm wondering what the best way to go about implementing that would be. adding an error out to it would make pre-existing playbooks fail under the condition, which is a con, so i'm thinking it would be better to add a warning, correct? or is that unfeasible with the project structure?
19:06:25 <nitzmahone> Did you see the PoC PR sivel added a bit ago?
19:07:02 <nytelife26> i did. it doesn't exactly address the issue at hand, though - it is related to another similar issue.
19:07:28 <nytelife26> unless you're suggesting i follow the same style, and implement it that way
19:08:38 <jborean93> My concern is changing the hostvars behaviour might break someone relying on the fact that it doesn't warn
19:08:50 <nytelife26> my primary concern was that just changing it to use fail_on_undefined would break playbooks that currently pass with that behaviour.
19:09:39 <nytelife26> yes, exactly. although it could also be argued that it is likely anyone using templating does not intend for it to pass with an unresolved template. is it possible to add an option to it, to enable fail_on_undefined optionally from the playbook?
19:10:50 <jborean93> without actually verifying this particular scenario, most things will fail if a template has an undefined variable. It seems like hostvars is a unique case here
19:10:52 <nitzmahone> What's weird is that the documented behavior seems to be that it'll fail anyway
19:11:06 <nytelife26> i do certainly understand his point, though. it's quite unexpected that something intended to be a template will just pass with no mention, warning or error.
19:11:18 <nitzmahone> I suspect the change to the `default` handling awhile back is what introduced this, but I'd have to debug it to be sure
19:11:32 <nytelife26> exactly my thoughts. it seems logical that it should fail. and yes, as far as my testing goes, it is unique to hostvars.
19:11:41 <nitzmahone> https://www.irccloud.com/pastebin/zvWqGGHs/
19:12:35 <nytelife26> so, the documented and decided behaviour is that it should fail?
19:13:12 <jborean93> for any other var that's what happens
19:13:21 <nytelife26> in that case, it makes sense that we should change it to suit the documentation, right? if that's a consensus i can implement that easily, i just wanted the thoughts of the rest of the team before implementing a breaking change.
19:13:56 <sivel> the PoC PR would be a replacement for you, but using the `vars` lookup instead of directly accessing hostvars, and due to it's explicit template call in the `vars` lookup, would do exactly what you want
19:14:01 <nitzmahone> It sure looks to me like the intended and documented behavior was for it to fail
19:14:26 <jborean93> I'm can't fully remember but potentially bcoca has mentioned `vars` acted like this and the recommended way was to use the `vars` lookup plugin instead
19:15:17 <nytelife26> alright. so, use vars lookup from the hostvars instead to make it follow the same behaviour as the rest?
19:16:10 <nitzmahone> Well, we can't change the`hostvars` object itself to never return a template, but off the top of my head, we should never be returning a raw template from Jinja
19:16:42 <nytelife26> that's what i mean. i'm not saying never return a template, i'm saying have it fail if the template doesn't resolve.
19:16:43 <nitzmahone> Sorry, I was confused by looking at the PR instead of the original issue (which has nothing to do with `vars` lookup)
19:17:19 <nytelife26> yeah me too, sivel seems to claim it could be implemented that way, though.
19:17:32 <nitzmahone> I don't think we can't do that at the object level though, because we *do* need that to return templates internally under some cirucmstances IIRC
19:18:25 <nytelife26> okay, i guess the best way to do this is to add an option to change the behaviour then, if both cases are required.
19:18:54 <nitzmahone> We may not be able to solve this right now in the meeting- will probably be some investigation required to see what the right path forward is
19:19:22 <nytelife26> internally, however, you can just pass an arg to disable it. afaik it'd be much harder to pass an arg to enable it from a playbook, right?
19:19:44 <nitzmahone> If our internal usages of that object don't require the "preserve the raw template" case, it's probably fine to just correct it inline, but if they do, we'll probably need to handle it somewhere else
19:20:07 <nytelife26> if i'm correct in that, it would logically be the optimal solution to default to failure, but add the option to preserve the raw template for internal functions
19:20:18 <nitzmahone> Yeah, the typical access via hostvars is an indexer, so we can't just add args
19:21:03 <nitzmahone> (nor would we want to add playbook syntax to support this)
19:21:05 <nytelife26> right. so if we can't add args for playbooks, is it easier to just do it for internal functions, though? and have it default to failure.
19:21:24 <nitzmahone> Probably
19:22:24 <nytelife26> alright. in that case, i'll research where it's needed to preserve internally and see if there's any more optimal solutions, but as far as i can see then, the best option is to change the default to follow documentation but add an override option for cases where preservation is needed. sound good, or?
19:22:42 <nitzmahone> Just fixing it to fail always *might* be fine, but there are a lot of layers to the varsmgr stuff, so if the playbook-facing layer that bombs gets accessed by something else that's not always ready to template things in a host context, "it would be bad" ;)
19:23:34 <nytelife26> that's why i'm saying it should have an override for other internal functions, just in case the templates need to be preserved and an error is not viable.
19:23:57 <nytelife26> then it could be fixed inline for places where it's needed so internal functions that require that preservation can just use the override arg
19:24:22 <nitzmahone> Yeah, if you're willing to try it, just change it to always fail, then add `ci_complete` somewhere in your last commit message and it'll force everything to run against it
19:24:54 <nitzmahone> That'd at least flush out any cases where it bombs with our existing tests, but I suspect we're not exercising all the possibilities
19:25:04 <nytelife26> alright. i'll give that a shot, and if it fails, i'll look into adding an override for the places that need it to pass
19:25:09 <nitzmahone> cool, thanks
19:25:34 <nitzmahone> #action nytelife26 to attempt the "make it always fail" case on hostvars template rendering failure
19:25:41 <nitzmahone> #topic open floor
19:25:51 <nytelife26> that's it for that issue for this meeting, then. thank you all :) anything else, or is that all for today
19:26:26 <nitzmahone> It's open floor, so if anyone else has something to discuss, we can, otherwise we'll wrap up in a couple mins
19:28:26 <jborean93> I'm good
19:29:21 <nitzmahone> OK, easy deal- 'til Thursday then... Thanks all!
19:29:27 <nitzmahone> #endmeeting