19:05:38 <shertel> #startmeeting Ansible Core Public IRC Meeting
19:05:39 <zodbot> Meeting started Tue Mar 22 19:05:38 2022 UTC.
19:05:39 <zodbot> This meeting is logged and archived in a public location.
19:05:39 <zodbot> The chair is shertel. Information about MeetBot at https://fedoraproject.org/wiki/Zodbot#Meeting_Functions.
19:05:39 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
19:05:39 <zodbot> The meeting name has been set to 'ansible_core_public_irc_meeting'
19:05:53 <felixfontein> let's see how many show up :)
19:06:08 <shertel> #info agenda https://github.com/ansible/community/issues/649
19:06:59 <sivel> O_O
19:07:00 * shertel pokes some people
19:07:12 <sivel> *ouch*
19:07:18 <felixfontein> :)
19:08:28 <shertel> #topic https://github.com/ansible/ansible/pull/77035
19:08:35 <shertel> felixfontein, this is yours
19:08:46 <felixfontein> ah, thanks
19:08:56 <bcoca> skipping the previous items?
19:08:57 <shertel> looks like it has approvals
19:09:08 <bcoca> ^ its on my queue for final review this week
19:09:09 <felixfontein> this one is about --metadata-dump, right now if some plugin has invalid documentation that fails parsing, the whole call will fail
19:09:12 <shertel> was waiting to see if more people showed up
19:09:38 <bcoca> its been updated since those approvals, so need to re rewview
19:09:46 <felixfontein> which I think is not a good idea or most uses of --metadata-dump (except when you use it for testing whether all your plugins/modules have valid documentation)
19:10:35 <felixfontein> I changed the option name as suggested, improved tests based on a comment, and rebased a couple of times (with no further changes)
19:10:36 <bcoca> felixfontein: cause that  is way to bar entry to those PRs that introduce plugins with broken docs
19:10:56 <felixfontein> bcoca: `ansible-test sanity` already makes sure that this doesn't happen
19:11:26 <shertel> That makes sense to me. Looks like it's just a new option, to keep backwards compatibility?
19:11:28 <shertel> bcoca would you be able to re-review? I can give it a review too.
19:11:39 <felixfontein> shertel: exactly
19:13:19 <felixfontein> bcoca: shertel: thanks for reviewing it (again)!
19:13:32 <shertel> felixfontein: anytime!
19:13:57 <shertel> #action shertel to review #77035
19:14:54 <shertel> #topic https://github.com/ansible/ansible/pull/76388
19:16:00 <shertel> this one was discussed a couple weeks ago, but it seems like it needs a quorum to vote on it. I haven't looked at it in more detail since last time.
19:16:42 <sivel> I am -1 on changing behavior, +1 on just adding docs
19:16:48 <bcoca> mkost lookups alreayd do this , and we added rule for the output
19:16:51 <sivel> I mentioned my backwards incompat concern
19:17:22 <felixfontein> I'm definitely against forcing this upon all lookups (that idea has been mentioned earlier as well). for this specific one, I think it is ok (in the sense that: I have to still think about whether I like this :) ) since the plugin fails if any arg is not a string.
19:17:25 <bcoca> sivel: can be done while still backwards compat, just need to start introducing deprecations or we keep the very complex input handling code (which many already do)
19:19:12 <felixfontein> sivel: is no longer failing on input that used to always fail validation a breaking change that should be avoided? wouldn't that also mean that you cannot add new options to a module, since when it was passed before it failed, and not it no longer does that?
19:19:19 <felixfontein> (or am I missing something here?)
19:21:07 <sivel> felixfontein: the issue is about doing `q('a_plugin', a_list)` instead of `q('a_plugin', *a_list)`.  Unpacking is required now, allowing to just pass a list where it would break people who actually intended to pass a list of lists to the lookup
19:21:38 <felixfontein> sivel: but the lookup will fail if you pass anything else than strings
19:21:54 <sivel> a specific lookup, may, not all lookups
19:22:18 <felixfontein> ah, so the question is whether to do it for *all* plugins?
19:22:28 <bcoca> sivel: since input type is defined 'lists of lists' is possible
19:22:44 <sivel> that was my intention.  I really hate lookups to begin with
19:22:53 <sivel> they are all so unique snowflakes
19:22:58 <bcoca> felixfontein: fixing the one plugin to align with others made me ask the question, should this be universal? since we also have rule that they always output a list
19:23:01 <sivel> they all do things differently
19:23:16 <felixfontein> I don't think this change should be made for *all* lookups, there are some that actually accept lists and that will get broken by this
19:23:18 <bcoca> sivel: agreed, lookukps are really 'functions'
19:23:39 <bcoca> felixfontein: not if properly define the input at list of lists
19:23:50 <sivel> I was advocating that we should do things for all lookups in a standardized way
19:23:55 <bcoca> also most of those lookups shoudl be filters
19:23:57 <sivel> and unpacking is the most standard today, and already works
19:24:01 <sivel> so just document it
19:24:07 <shertel> I'm -1 to the code change and +1 to just documenting argument unpacking. I'm not against the PR itself, but consistency is nice, and if this will break other plugins I'd be in favor of not supporting it piecemeal.
19:24:28 <bcoca> sivel: except we dont really unpack that way, its diff for many lookups, they aribtrarily take all args as string or unpacked
19:25:01 <bcoca> see password lookup (on my queue to fix)
19:25:12 <sivel> I mean at this point, they all suck, so it doesn't hurt us really to accept it either. lol
19:25:17 <bcoca> lmao
19:25:29 <nitzmahone> Yeah, I'd love to standardize on one pattern that's sane, and if we insist on supporting others, that it be opt-in with an attr on the function itself
19:26:01 <nitzmahone> But I don't know if it's worth the trouble
19:26:02 <bcoca> nitzmahone: righ tnow we support 12 diff ways ... this i think can let us normalize in one way they 'should all work' and eventually start deprecating the others
19:26:30 <bcoca> ^ main reason i 'normalize'd most core lookups to handle 'normal parameter unpacking'
19:26:35 <bcoca> but missed password
19:27:06 <bcoca> the main reason most accept lists is cause of with_ as that was the expected input
19:27:26 <bcoca> xcept first_found and fileglob that did ... horrible horrible things ...
19:27:41 <felixfontein> lol
19:27:41 <bcoca> well, still do, but now also allow unpacking
19:27:52 <bcoca> first_found should not even be a lookup
19:28:01 <bcoca> cartesiann and others .. really are filters
19:28:35 <sivel> I still think I'd just document unpacking, that's my vote.
19:29:21 <bcoca> not really an issue with unpacking/full string but do we want to force 'the ifrst input item to always be a list'
19:32:26 <shertel> I'd also be okay with making it opt-in via an attr on the function itself
19:33:09 * bcoca considers just killing lookups and start over with new 'plugin type' called 'query'
19:33:18 <shertel> +1
19:37:43 <shertel> Not sure if that's conclusive, but if there's nothing else on that topic....
19:38:05 <shertel> I think I'd need to implement the change generally + ensure there are tests for each lookup to see the extent of what would break to change my mind.
19:38:39 <shertel> #topic https://github.com/ansible/ansible/issues/16305
19:39:11 <shertel> doesn't look like rabin-io is around to discuss this one
19:40:01 <shertel> #topic Inventory as a generic construct (not just host)
19:40:03 <shertel> #info https://github.com/ansible/ansible/issues/50181
19:40:27 <shertel> this is one that came up in backlog and needed a vote
19:40:35 <bcoca> -1 to making the alias to 'target' ... i think most people can live with host: and not use it for actual hosts
19:41:19 <nitzmahone> Same
19:41:27 <shertel> Yeah, so they want to be able to use a generic keyword in the playbook instead of 'hosts'
19:41:31 <shertel> I'm also -1
19:42:19 <nitzmahone> There are plenty of other things we could/should do to make that sort of thing work better, but that's so far down the list...
19:43:18 <bcoca> to be fair, its an easy fix (add alias=target to field attribute for hosts:)
19:43:27 <bcoca> but ... not sure its worth it
19:43:49 <nitzmahone> Yeah, I'm not opposed to revisiting at some point, but there are much worse offenses in the "things that are poorly named" realm :D
19:43:51 <bcoca> i think it will create more confusion than it solves
19:44:02 <nitzmahone> --^ that's the bigger issue right now I think
19:44:03 <bcoca> run_once? roles?
19:44:35 <sivel> -1 to aliasing
19:44:50 <sivel> just accept that it's an identifier, and put whatever you want in there
19:45:01 <bcoca> k, think we all agree and can close issue now
19:45:24 <felixfontein> otherwise start using ansible-preprocess :)
19:45:48 * nitzmahone climbs in window
19:45:48 <bcoca> ansible-diy-semantics
19:45:55 <shertel> #agreed, no 'target' alias for hosts keyword
19:46:03 <felixfontein> convert the YAML to XML, use some generic XML transform, and convert it back?
19:46:22 <nitzmahone> If ASN.1 isn't involved somehow, it's not being done right ;)
19:46:23 <bcoca> missing toml, json and ini from that process
19:47:00 <sivel> I tried to implement toml playbooks, but just doesn't work since toml requires a top level dict and not top level list ;)
19:47:37 <shertel> #topic Open floor
19:47:51 <shertel> Anyone have anything else to bring up?
19:48:24 <felixfontein> how stable will the format of --metadata-dump be? is it safe to assume that it won't change substantially until the 2.13 release? like everything (globally, or for specific/all plugin types) moved into a subobject, or something like that?
19:50:59 <shertel> I'm not sure, but would assume that it won't change substantially
19:51:09 <bcoca> the general lstructure should not change, new fields appearing should be the biggest change possible
19:52:14 <felixfontein> sounds good to me
19:54:04 <shertel> Ok, thanks all for coming. I will comment on and close #50181.
19:54:15 <shertel> If there's nothing else...
19:54:18 <shertel> #endmeeting