15:08:50 <bcoca> #startmeeting ansible core public irc meeting
15:08:50 <zodbot> Meeting started Thu Mar  4 15:08:50 2021 UTC.
15:08:50 <zodbot> This meeting is logged and archived in a public location.
15:08:50 <zodbot> The chair is bcoca. Information about MeetBot at http://wiki.debian.org/MeetBot.
15:08:50 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
15:08:50 <zodbot> The meeting name has been set to 'ansible_core_public_irc_meeting'
15:10:57 * shertel waves
15:12:16 <bcoca> #topic https://github.com/ansible/ansible/pull/73707
15:12:28 <bcoca> implementation of https://github.com/ansible/proposals/issues/68
15:12:32 <felixfontein> \o/
15:12:33 <bcoca> looking for feedback
15:12:46 <felixfontein> andersson007_ wanted to take a closer look, no idea when though
15:12:53 <felixfontein> I'll also take a closer look, but not today :)
15:12:55 <bcoca> was thinking 'status: boolean' might not be best, 'supported: full/partial/none'
15:13:21 <bcoca> or go to proposal way having 2 sections features/limitations
15:14:12 <bcoca> felixfontein: also thought that instead of hardcoded list of 'desc', put it in the fragment itself, which allows for 'new ones' to be added easily, but some of these are planned to later be read by core
15:14:31 <bcoca> to give 'faster feedback' when keywords are used that are incompatible
15:16:08 <briantist> a little off-topic, but seeing the `facts` attribute reminded me of something I was thinking about a few months ago
15:16:42 <bcoca> well, we also have rule that _facts is part of action name that returns facts .. except setup ....
15:16:53 <felixfontein> (and hostname)
15:17:09 <felixfontein> bcoca: I haven't looked at the PR yet, so I'm not sure what you mean above
15:17:19 <bcoca> well, hostname returns facts as aftereffect, not as main purpose
15:17:26 <briantist> well what I was thinking abut was, for some/many `_info` modules, their information being returned as "info" vs "facts" isn't a clear cut distinction, and it would be useful if there was a way to say that you wanted an info module to return its information "as facts" (and vice-versa probably)
15:17:39 <bcoca> felixfontein: pr only implements it for copy , ansible-doc copy to see results
15:18:19 <bcoca> briantist:  basically register: is meant for that
15:18:23 <felixfontein> briantist: _info modules should never return facts
15:18:54 <bcoca> ^ we also want to change the 'facts automatically create top level variables' and always restrict them to keys under ansible_facts[]
15:19:07 <bcoca> the INJECT_FACTS setting will eventuially default to no
15:19:31 <bcoca> lots of security and memory issues due to the 'top level facts vars'
15:20:04 <briantist> I know the way things work now, and why :) but `register:` doesn't fully cover it setting the information, my thinking was around being able to use `_info` modules as one of the modules invoked by `gather_facts`
15:20:15 <bcoca> felixfontein: but hostname is a good exmple on why the 'facts' attribute it useful
15:20:40 <bcoca> briantist: you can, except they dont return the required ansible_facts
15:20:50 <briantist> right exactly :)
15:21:10 <bcoca> and i dont want 'any' module to run as gather_facts as they can have change impact on hosts
15:21:56 <bcoca> ^i have not restricted it 'yet' but it is something i have been considering
15:22:05 <bcoca> though it is a trivial thing to bypass
15:22:17 <briantist> I think I tried to futz around a little with an action plugin that would call the info module I wanted, and put the information into `ansible_facts`. It worked when called directly, but then realized it wouldn't work for `gather_facts` because it directly invokes the module, not actions
15:23:36 <briantist> I'm not necessarily saying any module should be able to be called that way, I do think there's a case to consider about some `_info` modules being useful as `_facts` as well, where it should be kept to an opt-in basis..
15:24:02 <bcoca> then those really should be facts modules
15:25:03 <bcoca> also, you really need to do hack at module level not at action plugin, some of the moved to _info still returned facts when called as _facts, you can check your own invocation name to make the change
15:25:03 <felixfontein> bcoca: btw, does INJECT_FACTS=no also make set_fact stop defining top-level vars?
15:25:24 <bcoca> felixfontein: set_fact and include_vars are not 'normal facts' and have their own hardcoded processing
15:25:49 <bcoca> also .. they dont really create facts
15:26:02 <bcoca> we just use same 'transport' and create host vars
15:26:25 <briantist> I think there are some blurred lines there that don't easily fit into the `_facts`/`_info` dichotomy, and they pretty much always become `_info` modules then. But I guess looking at it the other way, it'd be easy to convert those to facts, and make an `_info` equivalent that really is just an action, and pulls the info out of `ansible_facts` instead...
15:26:26 <bcoca> unless 'cacheable=yes' in which case set_fact creates both (yes, its a mess and confusing ... )
15:26:30 <felixfontein> right, include_vars is similar... so INJECT_FACTS=no essentially disables all (non-vars) plugins from setting/creating variables, or something which behaves roughly like them (assuming they don't do very dirty things)
15:27:04 <bcoca> felixfontein: it keeps them under anssible_facts (or ansible_local which is one exception to that)
15:27:46 <bcoca> #topic all about facts
15:27:53 <bcoca> we really are not on original
15:28:09 <briantist> ah.. sorry about the derail
15:28:34 <bcoca> np, i didnt expect much discussion on pr, just wanted to make it more public so people would think about it
15:28:44 <briantist> fwiw I really like the attributes :)
15:29:21 <bcoca> its somethign we've needed for a while to standarize how we explain 'quirky' module/action behaviour, isntead of having diff versions of it in descriptions/notes all over
15:29:53 <briantist> definitely, I remember when this came up in a previous meeting too. Big fan of more structured ways to refer to and reference that info
15:29:59 <bcoca> phase 2 will be reading from controller and dealing with the issue w/o having to execute the module on remote
15:31:02 <bobo175> as an Ansible user and (very) occasional contributor, I like the idea.  As long as https://docs.ansible.com/ansible/latest/dev_guide/developing_modules_documenting.html#documentation-block is updated w/ the new standards, I think folks like me will be happy to obey :-)
15:32:55 <bcoca> yep, just need to figure out what hte standard is first, the proposal and PR are not exactly the same and i'm still looking for diff use cases and optimization on the interface
15:34:00 <bcoca> my main 2 questions above a) status: boolean vs a more subtle supported: full/partial/none b) should list be cannonical in core or more flexible allowing for module/doc_fragment authors to expand even though those won't be 'meaningful' for p2
15:36:57 <bobo175> are there cases where partial is appropriate?  Seems like all the items in the proposal are binary.  Either the module DOES support it or it DOES NOT
15:37:40 <bobo175> if something "sort of" supports something, maybe it's not done yet?
15:37:43 <bcoca> check_mode/diff_mode have many modules with 'partial' implementation, specially diff mode is a lot harder in some casases
15:38:00 <bcoca> i would argue yes on check_mode, but its murkier on diff
15:38:00 <bobo175> oh yeah, diff mode is tough
15:38:14 <bobo175> I (lazily) never support diff mode
15:38:40 <bcoca> having a case where chekc_mode is not supported is almost a bug imho, but there are cases like 'updating repo mirror' in which there is no 'dry run' way to verify
15:39:04 <shertel> for check mode, we tend to return True for cases we don't know, so it might not be obvious which things are partial support without poking through the code
15:39:14 <bcoca> so installing packages almost always has check_mode, but repo updates .. depends on underlying tech
15:42:24 <felixfontein> check mode can often only be implemented partially as well, like having to return changed=True in some edge cases where maybe no real change happens because you cannot find that out without actually doing it (depending on the API you have)
15:43:01 <bobo175> yeah, there are certainly cases where an underlying API may not support a dry run.  I'm convinced this is a need for "partial"
15:43:43 <shertel> if a module defines a list of attributes, that overwrites the doc fragment, right? Rather than merging, since it's a list
15:44:10 <bcoca> they merge, see copy, for example it sets status: true on check/diff and adds version_added
15:44:30 <bcoca> any  items that are new on either side will also appear
15:44:38 <felixfontein> for top-level lists, they usually merge, like `notes:` and `description:` on top-level merge (and are sorted after appending, which results in 'fun' from time to time :) )
15:44:40 <bcoca> but descriptions are currently hardcoded .. which i might remove
15:45:44 <bcoca> in any case, i just wanted to introduce more people to thinking about this and will probably try to gather feedback in future irc meeting for when 2.12 PRs are mergable
15:47:04 <bcoca> #topic open floor
15:54:55 <bcoca> k, since we all seem out of gas, ending it
15:55:02 <bcoca> thanks for attending!
15:55:05 <bcoca> #endmeeting