19:01:12 <bcoca> #startmeeting ansible core public irc meeting
19:01:12 <zodbot> Meeting started Tue Jun 30 19:01:12 2020 UTC.
19:01:12 <zodbot> This meeting is logged and archived in a public location.
19:01:12 <zodbot> The chair is bcoca. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:01:12 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
19:01:12 <zodbot> The meeting name has been set to 'ansible_core_public_irc_meeting'
19:02:08 <felixfontein> #info agenda https://github.com/ansible/community/issues/541
19:03:50 <tremble> Almost time for a new issue
19:04:08 <bcoca> #topic https://github.com/ansible/ansible/pull/70060/
19:04:10 <felixfontein> only one day away :)
19:04:35 <bcoca> not sure what this fixes, the yaml anchors are all already resolved at this point, we are only dealing with python dicts
19:04:36 <felixfontein> this PR fixes a problem with the ansible-doc man page formatter
19:04:53 <felixfontein> the problem is that the formatter changes entries in these dicts
19:05:00 <felixfontein> like remove `description`
19:05:02 <bcoca> also makes adding new handling of fields require accounting in the new special var
19:05:10 <felixfontein> so if the next option is processed sharing the same dict, `description` is gone and it errors out
19:05:16 <bcoca> felixfontein: its on purpose to allow the 'general handler' to deal with 'what is left'
19:05:55 <felixfontein> bcoca: if you want to keep the code as close as possible to today, here's another PR: https://github.com/ansible/ansible/pull/70045
19:06:10 <felixfontein> it simply creates copies before starting to modify things
19:06:51 <bcoca> ^ thought we already got a copy .. we used to from plugin docs .. so something changed there at one point and we were getting a 'linked struct' instead of an unrolled copy
19:07:04 <felixfontein> no, we don't
19:07:08 <bcoca> i prefer going back to this, but see if using 'deepish' might be better option
19:07:16 <felixfontein> otherwise tadeboro wouldn't have had problems with YAML anchors :)
19:07:35 <felixfontein> you mean deep copy?
19:07:38 <bcoca> we didnt have issues originally, one of things i had manually tested (but ... didnt add test ...)
19:07:46 <bcoca> no, deepish ... its a thing, trust me
19:08:27 <felixfontein> the issue would also happen without suboption handling in case someone would be using YAML anchor for top-level options (not that someone should do that, but...)
19:09:01 <felixfontein> by recursively processing suboptions (and return values) this problem got easier to trigger
19:09:14 <bcoca> module_response_deepcopy < aka, 'deepish'
19:09:17 <shertel> https://github.com/ansible/ansible/pull/44337
19:10:00 <felixfontein> regular deepcopy won't help here anyway since it reserves YAML references
19:10:22 <felixfontein> module_response_deepcopy would work fine here
19:10:50 <bcoca> in any case, i do lean more for the 2nd option vs the PR that is title of this topic
19:11:11 <bcoca> i want LESS hardcoded lists in code .. not more ...
19:11:27 <bcoca> and the data structure reduction is a 'cleanish'
19:11:36 <bcoca> way to whittle down from known/handled to 'rest'
19:11:40 <felixfontein> in that case, a copy needs to be made somewhere
19:11:55 <felixfontein> shertel: thanks for the PR link btw!
19:12:25 <bcoca> yeah, dont know when that changed, but anchors used to work and that would have required a copy (though its been years since i wrote a module with anchors in docs ...)
19:12:30 <felixfontein> #70045 does the minimal amount of copying; though this isn't very time critical, so module_response_deepcopy would also work if you prefer that
19:13:31 <felixfontein> bcoca: for HTML docs anchors work fine, and for man docs suboptions were not modified, and anchors for toplevel options are not useful since they would end up with the same description - probably that's why it worked in the past
19:13:33 <bcoca> actually, add_fields should not be main 'popper' its the main loop on top level entries
19:13:44 * bcoca needs to reread the full code path
19:14:38 <bcoca> inside add_fields there really should not be any 'rest' .. the pop is mainly for main loop to allow users to add their own major sections and still get displayed (LABEL: whatevs)
19:14:52 <bcoca> aka how metadata is still displayed in 2.10 w/o specific code to handle it
19:15:15 <bcoca> also in previous versions .. but that was cause of bug that skipped metadata handling ...
19:16:19 <felixfontein> so. what do you propose?
19:16:25 <felixfontein> (for the PR?)
19:16:33 <bcoca> idk .. more coffee?
19:16:50 <bcoca> we do have catchall in add_fields .. but not sure we realy need/want it there43
19:17:03 <bcoca> allow users to add optional doc fields to plugins ?
19:17:14 * shertel isn't familiar enough with the code, would need to review
19:17:36 <felixfontein> bcoca: ok, but that doesn't really matter for this PR, right?
19:17:41 <bcoca> i know someone/somwhere will have  a use .. but this is very niche .. the top level 'user deffined docs' is already nieche, this is squaring that
19:18:12 <bcoca> makes a much simpler PR possible, just change to .get and remove 'rest handler'
19:18:53 <bcoca> remove 550-564
19:19:06 <felixfontein> right now `elements` isn't handled, and neither is `example` or `sample` for return values
19:19:15 <felixfontein> these need to be added then
19:20:13 <bcoca> cause now its not just documentation.options, but returndocs. that is added to add_fields call
19:20:47 <felixfontein> yes. but not in this PR
19:20:48 * bcoca really needs to cleanup a lot of taht code
19:21:04 <felixfontein> this PR is only a bugfix for making modules/plugins using YAML anchors in suboptions work again
19:21:42 * bcoca moves to xlst processing
19:23:31 <felixfontein> is there a reason not to merge this PR?
19:24:19 <bcoca> what i stated above, not a good implementation, prefer the other fix
19:24:43 <felixfontein> bcoca: I mean #70045 now
19:24:53 <felixfontein> (I got that you don't want another hardcoded constant list :) )
19:25:51 <felixfontein> it would be nice if this bug gets fixed for 2.10 in some way. afterwards you can refactor the code as much as you want :)
19:27:04 <shertel> Looks good to me
19:30:20 <bcoca> no one else on this topic?
19:30:49 <sdoran> I'm in favor of #70045.
19:32:49 <bcoca> k, think we can move on then
19:33:03 <bcoca> #topic https://github.com/ansible/ansible/pull/70026
19:33:10 <felixfontein> ok. what will happen to the 70045 next?
19:33:20 <shertel> I'm merging it
19:33:22 <bcoca> felixfontein: someone will review and merge
19:33:24 <felixfontein> shertel: thanks!
19:33:28 <bcoca> need to close the other one
19:33:39 <felixfontein> don't close the other yet, it has a test that can still be merged
19:33:54 <shertel> +1
19:34:02 <bcoca> then add tests to yours .. already closed
19:34:30 <felixfontein> I'll try to copy it over after the meeting
19:34:40 <sdoran> Yeah, that's a good test.
19:34:49 <sdoran> Would be nice to add it to #70045.
19:34:49 <felixfontein> about 70026: this adds the ability to the collection loader to know which collection a plugin comes from
19:35:06 <felixfontein> it can also distinguish between ansible.builtin, and plugins/modules from library/ etc.
19:35:22 <felixfontein> this is used to print the correct FQCN for a module in ansible-doc's man page formatter
19:35:23 <bcoca> ^ iirc there is much easier way to do this, we already have collection in ansible-doc
19:35:37 <felixfontein> bcoca: I don't think we do
19:35:38 <bcoca> felixfontein: also, not man page, that is diff program
19:36:13 <felixfontein> the function is called `DocCLI.get_man_text`, that's why I call it man page formatting
19:36:17 <bcoca> felixfontein: yes, we do, just need to look at redirect from plugin
19:36:56 <bcoca> felixfontein: function name is also incorrect,  it does 'try' to look like man page a bit, but really far from following the standards/format
19:37:29 <felixfontein> for me looking close enough is fine :)
19:37:51 <bcoca> ;-p
19:39:08 <felixfontein> do you mean result.redirect_list with "redirect from plugin"?
19:39:09 <bcoca> im working on update that fixes 'deep' reading of collecitons, you always get collection name with it, no need to go through all this work when iter_modules gives you the correct name
19:39:18 <bcoca> well, yes, that should ALSO have the name
19:39:24 <bcoca> but was going for pkg_name
19:39:42 <felixfontein> which pkg_name?
19:39:43 <bcoca> right now we are incorrectly reading symlinks as 'main file' when they are supposed to be aliases
19:40:06 <bcoca> the one the collection loader creates when it loads the collection resource aka pythnon package aka pkg_name
19:41:23 <bcoca> also avoids mutating some of the basic functions in loader to now return tuple
19:41:56 <felixfontein> when I run `ansible-doc file`  I get redirect_list == ['file']; looks exactly the same for a local plugin from a playbook
19:42:33 <bcoca> cause file should be the built in or library/, it wont use collection
19:42:33 <felixfontein> so how do I distinguish between ansible.builtin and plugin outside of collection and ansible.builtin?
19:42:57 <bcoca> soo ... builtin outside of collection?
19:43:43 <felixfontein> I thought that for builtin modules/plugins, it should show `ansible.builtin.xxx`
19:44:00 <bcoca> no, lets keep it naked, show colleciton for collections
19:44:03 <felixfontein> that would be the FQCN for a builtin plugin
19:44:05 <shertel> I thought we would keep the output the same for builtin
19:44:20 <felixfontein> ok, in that case it's pretty trivial to implement this
19:44:30 <bcoca> yeah,  .. now i get why your PR is like that, no ,... want to keep current plugins displaying as is, only fqcn for collectino content
19:44:47 <bcoca> felixfontein: that is what i was saying ... just didnt realize you wanted that ... which we really dont want
19:45:06 <felixfontein> I was somehow assuming that this was what we wanted... I guess I must have misremembered
19:45:09 <bcoca> thought you were trying to fix the c.general case about the symlinks
19:45:17 <felixfontein> (this was a couple of weeks ago... so now I remember even less)
19:45:20 <bcoca> which currently displays wrong name
19:45:27 <bcoca> though 'usable' ...
19:45:39 <bcoca> also that ansible-doc currently ignores routing.yml ... which is part of my fix also
19:45:48 <felixfontein> no, it still shows community.general.docker_container
19:46:06 <felixfontein> even though technically it would be community.general.cloud.docker.docker_container
19:46:11 <bcoca> #topic https://github.com/ansible/ansible/issues/70134#issuecomment-650223767
19:46:16 <bcoca> felixfontein: yes, that is part of what im fixing
19:46:32 <bcoca> cause right now its a hack of my doing an ls ~coll/plugins/modules/
19:46:39 <bcoca> and it needs to use colleciton loader
19:46:46 <bcoca> that is the 'iter_modules' function
19:47:00 <bcoca> which is also incomplete (TODO: read runtime.yml)(
19:47:03 <felixfontein> what would be the correct value then for docker_container? community.general.docker_container or community.general.cloud.docker.docker_container?
19:47:28 <bcoca> lthe 2nd, with the first appearing in 'ALIASES: ' field, just like symlinks do now with 'builtin'
19:47:54 <felixfontein> btw, I hacked together a tool which can convert between symlinks and runtime.yml entries (and do some more validation): https://github.com/ansible-collections/community.internal_test_tools/blob/main/tools/meta_runtime.py
19:48:08 <felixfontein> hmm. that sounds horrible.
19:48:22 <felixfontein> do we really want to show this long name to people?
19:48:37 <felixfontein> ok, I guess this is off-topic
19:48:40 <bcoca> not my choice
19:48:50 <bcoca> but people keep insisting on doing things certain ways ...
19:49:01 <felixfontein> flatmapping to the rescue :)
19:49:12 <bcoca> well, we both support and do not support flatmapping
19:49:19 <felixfontein> about https://github.com/ansible/ansible/issues/70134#issuecomment-650223767: it looks like the module_utils part of ansible_builtin_runtime.yml is screwed
19:49:23 <bcoca> depending on whom/how you ask
19:49:46 <shertel> Oh, yeah https://github.com/ansible/ansible/blob/devel/lib/ansible/config/ansible_builtin_runtime.yml#L7572-L7573
19:49:50 <shertel> :-/
19:50:10 <felixfontein> apparently during the migration it was assumed flatmapping is used for module_utils
19:50:11 <bcoca> felixfontein: afaik nitzmahone is taking care of that
19:50:17 <felixfontein> ok
19:50:26 <bcoca> well, during migration we were told flatmapping was a thing
19:50:39 <bcoca> its only changed 23423423 times after the migration ...
19:50:50 <felixfontein> it never was going to work for module_utils
19:51:09 <felixfontein> since things like `common` are used in multiple directories of stable-2.9's module_utils
19:51:58 <bcoca> imho, just need to update file to have full names
19:52:02 <nitzmahone> Yeah, I've got a couple of bugfixes in-flight for that and the "better failure when a redirected module_util can't be found", as well as fixing the 2.9 module_utils __init__ stuff
19:52:36 <felixfontein> nitzmahone: how do you repair the ansible_builtin_runtime.yml module_utils part? I hope not manually :)
19:53:34 <nitzmahone> Oh at this point it probably will be
19:53:49 <bcoca> it CAN be scripted ..
19:53:52 <nitzmahone> By the time I write a custom tool to backfill it, I'd be done
19:54:02 <felixfontein> pre-ansible-base had 542 files that weren't called __init__.py
19:54:35 <felixfontein> but maybe the information from BOTMETA.yml can help
19:54:43 <nitzmahone> The hardest part will be figuring out where they all went, and what to do about the partner/community split ones that probably got duplicated in many cases
19:54:47 <nitzmahone> perhaps
19:55:13 <felixfontein> true
19:55:15 <felixfontein> good luck with it!
19:55:24 <bcoca> felixfontein: botmeta is woefully out of date
19:55:50 <bcoca> it was 'abandoned' since days after it's update, bot stopped using it in favor of traversing galaxy to find matching files
19:55:50 <felixfontein> bcoca: I think most PRs updating ansible_builtin_runtime.yml also updated BOTMETA. or at least the ones I saw
19:56:00 <felixfontein> oh ok
19:56:07 <bcoca> felixfontein: and mnay didnt  cause we told people to stop updating botmeta
19:56:13 <felixfontein> maybe it should be deleted then?
19:56:15 <bcoca> only those we told before the change, did
19:56:16 <nitzmahone> The fix for the one you're talking about (supporting dotted source names for module_utils) is already working- the only part that's not is the empty-package synthesis for the collections redirected cases
19:56:37 <bcoca> nitzmahone: and iter_modules doesnt read runtime.yml
19:57:02 <bcoca> though it really only needs to do so to get aliases, but that requires a reversal of current strcuture
19:57:22 <nitzmahone> bcoca: I think I've still got a branch laying around somewhere where it does, but yeah, I'm not sure we want it to in most cases, and doing it conditionally is ... well, no ;)
19:57:23 <bcoca> and also how to deal with symlinks correctly
19:57:33 <felixfontein> btw, in case you need test material, community.general 0.2.0 uses symlinks, while 0.3.0-experimental.meta.redirects uses meta/runtime.yml for redirects
19:57:47 <bcoca> nitzmahone: we need to to present 'correct info' in docs
19:57:53 <nitzmahone> (which are nonfunctional on 2.9, I assume)
19:58:10 <bcoca> no, symlninks work in 2.9, runtime.yml doesnt
19:58:23 <bcoca> they also work in 2.10 .. they just look like 'main' not 'alias' in collections
19:58:35 <felixfontein> that's why 1.0.0 will probably keep symlinks, the 0.3.0 prerelease is only for testing how well meta/runtime.yml redirects work
20:00:28 <nitzmahone> yeah, it'll be interesting to see how many people want to keep using stuff in 2.9, and what kinds of practical problems that will present us (I know of a few already, but I'm sure there are others)
20:00:52 <nitzmahone> For 2.10, probably not a big deal, but the more things drift, the more "interesting" that will likely become
20:01:08 <felixfontein> indeed
20:01:14 <bcoca> nitzmahone considering that all other products are staying on 2.9 ....
20:01:37 <nitzmahone> bcoca: exactly
20:01:47 <felixfontein> which products do you mean?
20:02:00 * bcoca wishes he had gone forward with betting pool, would be raking it in right now
20:02:27 <bcoca> felixfontein: anything 'officially supported' is standarizing on 2.9
20:03:03 <felixfontein> for how long? until ansible-base 2.10 is there, or much longer?
20:03:34 <bcoca> k, at this point i think we can continue that chat elsewhere, we are far from meeting topics and over time at this point
20:03:37 <bcoca> #endmeeting