15:03:41 <sdoran> #startmeeting Ansible Core Public IRC Meeting
15:03:41 <zodbot> Meeting started Thu Jul 22 15:03:41 2021 UTC.
15:03:41 <zodbot> This meeting is logged and archived in a public location.
15:03:41 <zodbot> The chair is sdoran. Information about MeetBot at http://wiki.debian.org/MeetBot.
15:03:41 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
15:03:41 <zodbot> The meeting name has been set to 'ansible_core_public_irc_meeting'
15:04:34 <sdoran> Just one item on the agenda today.
15:04:44 <sdoran> #topic https://github.com/ansible/ansible/pull/75247
15:05:15 <cyberpear> I've definitely hit that bug before
15:06:27 <cyberpear> or rather, I considered that not a bug, but the new behavior a bug... IIRC there's some other similar filter to this that returns empty string vs false with no match
15:06:30 <sdoran> Not many in attendance today.
15:08:22 <felixfontein> I would suggest to not merge this in a bugfix release, but keep it for 2.12, and label it as a `breaking_change`, since this can change behavior for some people
15:08:23 <mkrizek> cyberpear: what is "the new behavior"?
15:09:35 <cyberpear> mkrizek: in the current behavior, you can distinguish between "no match found" and "matched empty string"
15:10:14 <cyberpear> (within the same jinja template, obviously)
15:10:55 * cyberpear looks for issue to see what problems it's causing
15:11:17 <bcoca> 'new behaviour' is 'consistent'
15:11:21 <bcoca> no matter the context
15:11:27 <cyberpear> yeah, I know for new things we don't want variant return types
15:11:29 <bcoca> vs current which changes '' or None depending on context
15:11:47 <bcoca> well, variants are ok if documented, many filters /lookups do this
15:11:53 <bcoca> the problem is tha tit is contrary to doc
15:12:24 <sdoran> #chair bcoca
15:12:24 <zodbot> Current chairs: bcoca sdoran
15:13:18 <sdoran> And the problem is if we change it for this filter we will need to change it for others.
15:14:00 <sdoran> Returning `None` is more Pythonic, but may not make as much sense to a non-programmer.
15:14:19 <tadeboro> I find current behavior more useful for the same reasons as cyberpear, but I think in this case consistency is more important since not all ansible users are programmers.
15:15:18 <cyberpear> the bug doesn't describe the issue, only describes the behavior as not matching docs
15:15:31 <cyberpear> i.e., what broke as-is?
15:16:23 <cyberpear> how many other instances do we return `None` but don't document it as such?
15:16:26 <bcoca> using it with  another filter that expected a string
15:17:15 <sdoran> Or a comparison to an empty string.
15:17:16 <bcoca> we would have to look into it, the 'None to '' ' is a sideeffect on how we process jinja2, so if its 'final result' you get '' (as documented) but as 'intermediate result' the None persists
15:17:27 <sdoran> Using `truthy` or `falsy` tests would be better.
15:17:27 <mkrizek> cyberpear: why would you "match empty string"? isn't an empty string contained in all strings?
15:18:36 <bcoca> result |type_debug == 'string'
15:18:44 <bcoca> many ways it breaks
15:18:45 <cyberpear> mkrizek: you'd think so, but when using regex lookaround to parse out a value that could be an empty value but the lookaround part matched
15:20:38 <cyberpear> I'd suggest that whatever the decision here be documented as a TODO for any other mis-matched filters
15:21:12 <sivel> so something I brought up during triage, is that this kind of breaks the ability to determine if the pattern didn't match, vs the pattern matching an empty string
15:21:16 <sdoran> We can either 1) Update the docs or 2) Update this filter to return an empty string (and probably several others).
15:21:44 <sdoran> Good point. `None` potentially provides more information than `''`.
15:22:43 <sivel> _finalize is just evil
15:22:55 <cyberpear> sivel++
15:25:49 <sivel> I do realize how complicated it makes documented the behavior of a filter like this, but I also still believe the current behavior of `regex_search` shouldn't be modified
15:26:38 <bcoca> im 50/50
15:26:49 <bcoca> and its really not the filter's fault  but our postprocessing
15:27:48 <felixfontein> that postprocessing tripped me up more than once
15:28:01 <mkrizek> now I am 50/50 too and I think the best solution here is to ignore it ;-)
15:28:05 <felixfontein> but it's hard to get rid of
15:28:40 <felixfontein> I think that in this case, improving the docs is better than changing the behavior
15:29:04 <sdoran> I'm in favor of updating the docs and keeping the behavior.
15:29:50 <felixfontein> an advantage of that is also that it's not a breaking change :)
15:29:50 <sivel> Note that we're also pushing toward a future of jinja2 native being a forced requirement
15:30:03 <cyberpear> is this something helped by the native_types `ANSIBLE_JINJA2_NATIVE`?
15:30:28 <sivel> cyberpear: yes, jinja2 native bypasses the None -> "" translation in Templar._finalize
15:31:06 <sivel> and we're planning to drop `safe_eval`, and transition to required jinja2 native
15:31:27 <tadeboro> If the native types are what is seen as the way forward, then documenting is definitelly the better option here.
15:32:18 <tadeboro> Although jinja's native type support has its own set of "interesting" behavior ...
15:32:24 <bcoca> change docs to 'return None' and then add docs about 'non native and none'
15:33:09 <cyberpear> is safe_eval what tries to avoid templating stuff that came from a managed node?
15:33:58 <sivel> cyberpear: `safe_eval` is what takes the pythons tring repr from jinja2 (non-native) and converts it into Python
15:34:05 <sivel> python string*
15:34:06 <tadeboro> safe_eval is what makes it possible to do things like `loop: "{{ something that returns list }}"`.
15:34:14 <mkrizek> cyberpear: https://gist.github.com/mkrizek/dbcf415b485fc3f2d4b3676ce0013397
15:34:17 <cyberpear> thx
15:34:40 <bcoca> j2native does the same ... but better
15:35:24 <tadeboro> Just for reference: https://jinja.palletsprojects.com/en/3.0.x/nativetypes/
15:35:34 <cyberpear> okay, so this is the sometimes-annoying "if it looks like a json string, parse it out to a datastructure" funcitonality
15:35:46 <tadeboro> cyberpear: Yep ;)
15:36:07 <tadeboro> cyberpear: But used in even more places.
15:41:07 <sdoran> Sounds like we have reached a consensus to keep the existing behavior but update documentation.
15:41:32 <sdoran> Is that correct?
15:41:51 <mkrizek> +1
15:41:54 <cyberpear> +1
15:41:59 <felixfontein> +1
15:42:23 <sdoran> #topic Open floor
15:44:23 <sdoran> I'll end the meeting shortly if there is nothing further to discuss.
15:50:15 <sdoran> Thank you everyone for participating today.
15:50:17 <sdoran> #endmeeting