16:00:30 <shertel> #startmeeting Ansible Core Public IRC Meeting
16:00:30 <zodbot> Meeting started Thu Jan 12 16:00:30 2023 UTC.
16:00:30 <zodbot> This meeting is logged and archived in a public location.
16:00:30 <zodbot> The chair is shertel. Information about MeetBot at https://fedoraproject.org/wiki/Zodbot#Meeting_Functions.
16:00:30 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
16:00:30 <zodbot> The meeting name has been set to 'ansible_core_public_irc_meeting'
16:00:35 <shertel> #info agenda https://github.com/ansible/community/issues/661
16:00:45 <felixfontein> o/
16:00:59 <shertel> hello! hope you are feeling better
16:01:26 <shertel> #topic https://github.com/ansible/ansible/pull/79370
16:01:27 <felixfontein> yes, I'm pretty fine, I had a cold, so nothing too bad...
16:01:43 <felixfontein> not good enough to go outside, but good enough to work and live :)
16:02:01 <shertel> I'm glad it's not bad
16:02:35 <shertel> I was just looking over this one again, looks like all my requested changes were made.
16:03:16 <felixfontein> I'm curious whether this has a chance of getting merged soon :)
16:03:20 <shertel> --metadata-dump still includes the private plugins, but no longer listed otherwise
16:03:49 <shertel> I just hadn't gotten around to reviewing it yet, it's been on my to-do list for a while
16:04:10 <shertel> LGTM, I will merge later unless anyone objects
16:04:13 <felixfontein> that's intentional, as the plugins still exist, so tools showing the documentation (which use --metadata-dump) should have access to it
16:04:30 <felixfontein> cool, thanks a lot! once its merged I'll merge an antsibull-docs PR which uses that data ;)
16:04:54 <felixfontein> I didn't want to merge it until its clear that ansible-doc will also use this flag
16:05:39 <shertel> bcoca, do you want a chance to review again too^ ?
16:08:47 <shertel> #action shertel to review/merge https://github.com/ansible/ansible/pull/79370
16:09:09 <felixfontein> thanks!
16:09:10 <shertel> #topic https://github.com/ansible/ansible/pull/77737
16:09:13 <bcoca> im still not convinced we need this
16:09:14 <sivel> oh, hey, forgot about this thing
16:09:34 <bcoca> i think documenting the description as 'internal use only'  is a good convention and there is no need to 'codify' it
16:09:59 <felixfontein> sivel: the meeting, or the PR?
16:10:03 <sivel> meeting :)
16:10:29 <shertel> I am +0 on it, have mostly just been relaying other people's reviews
16:10:54 <shertel> (on codifying 'internal use only')
16:11:05 <sivel> shertel: maybe bring it up in next weeks core team meeting just to ensure we do want to move forward with it, before merging :)
16:11:20 <felixfontein> I think it's important to make sure that users are not unnecessarily exposed to internals, and I did collect some other opinions (see the community topic)
16:11:23 <shertel> ok, this was the first complaint I heard
16:11:29 <bcoca> there is also the scenario .. don't you want options documented/readable for the 'internal people'? don't assume author is always around (or even remembers) what the plugin does/how it works
16:11:48 <sivel> I am basically +0 too. There have been a fair number of requests for it
16:11:57 <felixfontein> bcoca: you can still see the docs, but not everyone will see it right away
16:12:00 <bcoca> felixfontein: 'community topioc'?
16:12:20 <felixfontein> bcoca: https://github.com/ansible-community/community-topics/issues/154
16:13:43 <bcoca> for one, using the file name has strong opposition in core
16:14:04 <bcoca> _ is also overloaded with 'deprecated' .. though that does not apply to collections anymore since 2.14
16:14:08 <felixfontein> yeah, I don't mind not using the filename, that was just the easiest thing I came up with :)
16:14:24 <bcoca> he, why _deprecated was imp;lemented in 1.8 ...
16:14:38 <felixfontein> _ deprecation never really worked for collections
16:14:52 <sivel> I think we can move on to 77737
16:15:01 <shertel> #action shertel to bring up https://github.com/ansible/ansible/pull/79370 at next internal core meeting for more discussion
16:15:27 <bcoca> +1 on that
16:15:27 <shertel> felix is this one just waiting on a review too?
16:15:39 <felixfontein> shertel: yes it is...
16:16:15 <felixfontein> I think this one shouldn't be controversial, it's just doing for test and filter plugins what the tests do for all other plugin types, namely check that ansible-doc doesn't choke on the docs
16:16:51 <shertel> I can add it to my list, I might not get to it this week so if anyone else wants to merge sooner please do
16:17:03 <felixfontein> (having a schema test for the docs would be better, but that's more work...)
16:18:26 <shertel> I'll keep that in mind when I review it
16:18:44 <felixfontein> thanks again :)
16:18:55 <shertel> np
16:18:57 <shertel> #topic https://github.com/ansible/ansible/pull/79471
16:19:23 <felixfontein> this one is controversial I think :)
16:20:00 <felixfontein> I think somewhere (I don't remember anymore where) it was mentioned that core team never wanted the env variable fallback for modules in the first place
16:20:05 <bcoca> cause it is confusing about env vars, most people expect to set them on controller and then the modules pick them up (which they do if they are localhost or 'fake remotes' like networking)
16:20:39 <felixfontein> that's definitely true, but the same problem is also true for modules that read files...
16:20:45 <bcoca> and the current implementation which is via 'fallback to generic function, just happens to be the only one implemented is  lookup env var ..'
16:21:31 <felixfontein> I still think it's good to be able to document such fallbacks in a uniform way; in particular this allows the docs generation to add a hint that these vars must be defined in the correct environment
16:21:47 <felixfontein> (which you don't want to do manually in every description where you have to manually write which env var is used as a fallback...)
16:22:07 <bcoca> i would just rewrite the callbacks and allow for direct env= declaration in argspec .. but that is just me
16:22:21 <bcoca> making clear they are low precedence
16:22:39 <felixfontein> but that's orthogonal to them being allowed in the docs
16:22:40 <bcoca> and env being a list
16:23:18 <bcoca> felixfontein: not really  https://github.com/ansible/ansible/pull/79720 since  i hope to merge this for 2.15
16:23:32 <bcoca> docs/arsgpec will be much tighter
16:23:59 <shertel> oooh, hadn't seen that yet
16:24:23 <felixfontein> hmm, I'll have to look into that in more detail
16:24:42 <felixfontein> it still will take a long time for collections to actually use that
16:24:55 <bcoca> its mostly adding 'required_X' to docs, but if we add env= .. .that also should be added there since it also converts 'docs to argspec' and viceversa
16:25:12 <bcoca> so action plugin will now be able to use 'argspec' directly w/o hardcoding/duplicating
16:26:05 <bcoca> felixfontein: not really, some are already using their 'home grown' version  https://github.com/ansible/ansible/pull/79713 <= this si attempt to have validate-modules ignore their hack
16:27:20 <felixfontein> ah, I haven't seen that one yet either... I really like having this information in DOCUMENTATION, but mainly so that antsibull-docs can show it
16:27:36 <bcoca> exactly!
16:27:54 <felixfontein> didn't they have a similar PR some time ago (by another person)?
16:28:07 <bcoca> `we want a format not only to equate to the argspec, but be readable by users` <= my response to that pr
16:28:27 <felixfontein> yeah, that would be nice
16:28:40 <bcoca> felixfontein: yes, also diff implementation (both that started from some code i wrote around 2.2 to do conversion from doc to argspec and back)
16:28:55 <felixfontein> well, the version from argspec can be made readable by extra code
16:29:04 <bcoca> felixfontein: also why 'diff ' and 'check mode' made it into attributes
16:29:11 <felixfontein> I think russoz will also be very much interested in your PR
16:29:26 <felixfontein> I guess in that case 79471 is tabled for now
16:29:34 <shertel> okay
16:29:38 <shertel> #topic https://github.com/ansible/ansible/issues/79695
16:29:40 <shertel> last one
16:29:53 <felixfontein> yet another 'allow to hide stuff' PR ;)
16:29:58 <bcoca> i've been posting it and asking for feedback since novembre
16:30:12 <felixfontein> bcoca: I must have missed that...
16:30:51 <bcoca> i used to be a branch that iwas posting ... mostly ignored, so i created draft pr as it is easier to show diffs
16:31:13 <felixfontein> 79695 is important for sanity testing modules that also have an action plugin and uses some parameters to communicate between action plugin and module that shouldn't be used by users
16:31:23 <bcoca> felixfontein: iirc you are only one that actually gave feecback when i did ...
16:31:44 <shertel> this one seems like a reasonable request, we should still be able to validate private options
16:31:46 <shertel> +1
16:32:08 <felixfontein> also more importantly not having to disable certain tests ensures that these still run for new public options
16:32:18 <shertel> exactly
16:32:18 <bcoca> my issue with that, users can still pass the argument themselves
16:32:30 <bcoca> it will depend on action plugin if that is passed through or overwritten
16:32:31 <felixfontein> otherwise PRs might add new options without docs and they get accidentally merged because the sanity tests don't complain
16:32:52 <felixfontein> bcoca: true, but that's already the problem right now
16:33:26 <bcoca> not saying it isnt, saying that we should implement filter and not give the option of passthrough if the argument is really private
16:33:35 <felixfontein> the proposal won't change anything with regard to that, it will only make sanity testing more reliable for such module/action plugin pairs
16:33:46 <bcoca> the problem with filter ... it requires my PR first ...
16:34:07 <bcoca> since right now controller is oblivious to argspec
16:35:31 <felixfontein> is your main objection that this might be a breaking change when this is implemented before your PR?
16:35:33 <bcoca> that said, i don't see it as blocker
16:36:08 <bcoca> no, just sayign its an issue we'll need to address ALSO .. but requires more infra (which my PR provides) to be able to do so
16:37:04 <felixfontein> is the general idea (having a `undocumented` boolean flag in argspec) ok? if yes I can start with a PR for that (to make validate-modules aware of that)
16:37:35 <bcoca> bikeshed moment: internal=yes
16:37:42 <felixfontein> :D
16:37:51 <bcoca> since we also want filtering, undocumented might be 'incomplete name'
16:38:04 <felixfontein> I agree `internal` is better when there is also filtering involved
16:38:07 <bcoca> if we flag as internal, we can justify removign 'user input' later on
16:38:16 <felixfontein> I mainly chose `undocumented` because I didn't expect filtering to happen anytime soon
16:39:00 <bcoca> 'soon' is realtive ...
16:39:19 <felixfontein> more precisely, I didn't expect anything to happen for at least a year or so
16:39:36 <bcoca> i hope you are wrong ... i fear you might not be
16:39:40 <felixfontein> but then I didn't knew about your PR / branch (or if I did I forgot about it again...)
16:39:52 <bcoca> fair enough
16:40:08 <shertel> using `internal` would keep our options open for the future
16:41:21 <felixfontein> when we are in bikeshedding mode: should https://github.com/ansible/ansible/pull/79370 also use 'internal' instead of 'private'?
16:41:30 <bcoca> one thing we could also do  is change the ignore to be specific `validate-modules:undocumented-parameter:is_file`
16:41:59 <felixfontein> that might require a lot more code
16:42:12 <bcoca> felixfontein: yes, but again, think we dont need to codify this .. private also implies more 'secret/secure' than internal does
16:42:26 <felixfontein> or maybe not... might be a good Q for mattclay
16:42:34 <bcoca> felixfontein: yes, but the advantage of changing the test, you avoid changing runtime code
16:42:49 <bcoca> and having to change moduels/argspec/docs/doc printing
16:43:23 <felixfontein> the docs printing is not affected since it doesn't have access to the argspec (except with your PR, but it could filter that out)
16:43:29 <bcoca> which allows us more time to add my PR and then open the floodgates to changes that requried controller to have argspec knowledge (i.e validate module params at action plugin time!)
16:44:50 <felixfontein> ok for me, assuming that mattclay doesn't object
16:45:21 <bcoca> not saying that 'is the way' <voice=helment resonance> .. but that it might be an alternative
16:46:11 <felixfontein> it's generally a good thing, though, not just for this application
16:46:47 <felixfontein> sometimes you have screwed up things in the docs and have to keep it for some time for compatibility purposes, then at least you can make sure that sanity tests will catch the same error in another option
16:47:38 <felixfontein> (though in most cases the error is 'syntax error' and very unspecific... so it won't help in these cases)
16:48:07 <felixfontein> (syntax error because schema validation failed, not because invalid YAML)
16:50:23 <shertel> felixfontein: do you need anything else with this one? Not sure what the implementation looks like, but seems reasonable to me.
16:51:15 <felixfontein> shertel: you mean 'extended'/hierarchical error codes? I'd mainly like a quick opinion from mattclay what he thinks, but besides that I think I'm happy
16:51:37 <shertel> it might be a little early for him, I'm sure he'll respond when he sees it
16:51:47 <felixfontein> but this isn't urgent at all, it's mainly that this discussion came up again recently and I decided to create a proposal for it :)
16:52:02 <felixfontein> yeah, that's totally fine. thanks!
16:52:13 <shertel> sorry there is not more of a quorum here!
16:52:22 <bcoca> we are running out of time, so im not going to bring up the issue of 'meeting format change' .. but it might be something to discuss, specially with timezones limitations
16:52:59 <bcoca> somethine more async will make meetings slower, but increase participation, visibility and quorum
16:53:16 <felixfontein> waiting up to a month for the next meeting isn't exactly fast either :)
16:53:34 <felixfontein> so in average it will make things go faster, I think
16:53:44 <bcoca> agreed, but the byweekly was clearly too often and mostly cancled
16:54:03 <felixfontein> monthly wasn't that busy either
16:54:09 <bcoca> idk what best solution is, just seeing the current one is not working
16:54:21 <shertel> in general I'd prefer just to get pings directly in #ansible-devel/github when someone wants my opinion/a review
16:54:49 <bcoca> part of me wants to go back to ansible/proposals ... but that is also mixed since there is no onus to make people comment nor timeframes attached
16:55:06 <shertel> I'd be in favor of trying ansible/proposals again
16:55:08 <bcoca> ^ direct pings work for 'when SME is online and available'
16:55:31 <felixfontein> shertel: that's fine for uncontroversial stuff / 'simple' PRs, but I think for proposals / controversial PRs I'm not sure direct pings is a good idea
16:55:53 <felixfontein> ansible/proposals SGTM
16:55:58 <bcoca> quorum is hard since we are distributed across many timezones, why 'direct ping' cannot scale
16:56:20 <felixfontein> I don't even know who is in core team right now :)
16:56:55 <felixfontein> well I know (or well, mostly assume) some folks are, but I don't know if there are more
16:57:00 <bcoca> i have rough idea .. but not 100% sure myself ...
16:57:05 <shertel> lol
16:57:10 <felixfontein> :)
16:57:17 <bcoca> ^ not kidding,
16:57:26 <felixfontein> reminds me a bit of $JOB....
16:57:50 <bcoca> some people have a 'nebulous status' like nitz .. he is both core and not core, he is heisen-core
16:58:59 <shertel> yeah, I guess I do not count him exactly as core but part of a superset
16:59:00 <felixfontein> sounds like core team is a bit of a probability cloud
16:59:41 <bcoca> fyi, im out next week, but might still pop in
16:59:58 <bcoca> and every other week till mid march
17:00:12 <shertel> enjoy!
17:00:12 <felixfontein> note to self: try to get all the PRs merged that bcoca doesn't like
17:00:32 <shertel> thanks all for coming to discuss :)
17:00:34 <shertel> #endmeeting