15:02:43 <bcoca> #startmeeting core public irc meeting
15:02:43 <zodbot> Meeting started Thu Jun  4 15:02:43 2020 UTC.
15:02:43 <zodbot> This meeting is logged and archived in a public location.
15:02:43 <zodbot> The chair is bcoca. Information about MeetBot at http://wiki.debian.org/MeetBot.
15:02:43 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
15:02:43 <zodbot> The meeting name has been set to 'core_public_irc_meeting'
15:03:05 <bcoca> #topic https://github.com/ansible/ansible/pull/69313
15:03:53 <felixfontein> mattclay still needs to look at it
15:04:05 * resmo o/
15:04:11 <bcoca> i was going to say .. didnt we already punt this from core irc meeting?
15:04:20 <felixfontein> yes, twice I think :)
15:04:29 <felixfontein> I guess someone has to check it
15:04:41 <shertel> Oops... I didn't cross it off
15:04:44 <bcoca> i have, wasnt running the prev ones
15:04:48 <bcoca> #topic https://github.com/ansible/ansible/pull/69796
15:04:55 <felixfontein> shertel: no problem, this way we're not forgetting about it ;)
15:05:31 <bcoca> forget about what?
15:05:41 <felixfontein> that it's still alive
15:05:45 <bcoca> what is?
15:05:55 <felixfontein> for the PR in topic, https://github.com/ansible/ansible/pull/69454 needs to get merged first
15:06:09 <felixfontein> but we can still talk about the question whether return docs YAML parsing should result in an error, or in a warning
15:06:19 <felixfontein> bcoca: the antsibull-changelog PR
15:07:04 <bcoca> i dont remember
15:07:35 <bcoca> so , for 'other fields' we can ignore, but documentation and returndocs are required to be yaml
15:07:59 <bcoca> should both follow same path (made note in pr)
15:08:15 <bcoca> also, version striping, should not happen on ansible-doc side, we probably want it sooner
15:08:24 <felixfontein> right now, returndocs isn't parsed
15:08:27 <bcoca> ansibvle-doc should just handle 'formatting' in a light way
15:08:40 <bcoca> it is , just in the wrong place
15:08:48 <bcoca> well, x2
15:09:01 <felixfontein> it is never a failure though, it's always treated like "try to parse, if fails, keep as string"
15:09:03 <bcoca> its parsed as string in doc reading lib, then as string or yaml on ansible-doc
15:09:34 <bcoca> yep, and that was 'right in 2.2' when i was making the standard up .. currently and for a looong time we have required yaml, so fine with making that change
15:09:57 <felixfontein> ok. I'm also fine, but I guess this needs to be officially decided
15:10:02 <bcoca> fine even if empty, but if supplied it should be yaml AND the structure we defined
15:10:31 <bcoca> also remove metadata from our tests
15:10:42 <felixfontein> (your PR removes metadata)
15:10:53 <bcoca> and you add it in this one to test
15:12:14 <felixfontein> I added it in this PR because in the version of devel this branch is based on, there still is metadata
15:12:18 <felixfontein> I will remove it on rebase
15:12:23 <bcoca> roger
15:12:23 <felixfontein> once your PR is merged
15:12:28 * bcoca goes merge
15:14:28 <bcoca> didnt add comment, but still think we shoujld not do the version tagging  in doc/cli.py .. but pretty sure you did that since you were leaving 'non yaml ' returndocs as warning when parsed
15:15:36 <felixfontein> doc/cli does untagging so that the current collection name isn't mentioned in versions and dates
15:20:30 <bcoca> anything else, anyone else?
15:21:29 <bcoca> #topic https://github.com/ansible/ansible/pull/64474
15:21:39 <felixfontein> is a decision needed to fail on bad return docs (i.e. non-YAML), or can I just change it?
15:23:17 <cyberpear> o/
15:23:37 <felixfontein> (I'm happy with just writing `bcoca said so` as a reference.)
15:23:41 <bcoca> felixfontein: i gave my opnion .. no one else seems to agree/disagree
15:23:55 <bcoca> felixfontein: fine with me, but that is not guarantee of acceptance of the PR
15:24:04 <shertel> I still sort of prefer the warning, but I don't really care
15:24:05 <felixfontein> shertel: sdoran: any opinion?
15:24:20 <felixfontein> (or anyone else?)
15:24:22 * sdoran thinking
15:26:46 <bcoca> ^ we can revisit later, new topic
15:27:17 <bcoca> i gave 5 mins for thoughts before, will give some more again if we finish this topic and open floor
15:27:19 <sdoran> Yeah, I'm not able to offer a strong opinion on that.
15:27:32 <sdoran> Moving on.
15:27:59 * bcoca needs to write filter/tests docs parser
15:29:04 <sdoran> For the `regexp_findall` PR, I find the example to be curious since it seems it would be much simpler to use the `getent` module for parsing `passwd` entries.
15:29:58 <cyberpear> I can get a better example
15:31:54 <felixfontein> how about calling the option not `groupdict` but `groups_as_dict` or `as_dict`? or even `group_dict`?
15:32:22 <bcoca> groupings .. that it comes in a dict is less material to the name of the option
15:32:29 <bcoca> just that you return them
15:32:49 <bcoca> though to be consisten with most regex functions .. should it be a list?
15:33:00 <cyberpear> sure. I just reused the python option name
15:33:31 <sdoran> I was wondering about how changing a flag changes the output.
15:33:45 <sdoran> Not sure that's a good idea. Should probably return a consistent type.
15:34:13 <felixfontein> it will definitely make writing docs for this challenging
15:34:33 <cyberpear> sdoran: here's a real-world use https://github.com/jamescassell/ansible-role-nss-module/blob/6fd23ea8fbfd57ebac3640cb91f7614b2e9a505f/vars/main.yml#L4-L7
15:35:35 <bcoca> actually, findall already returns a list of groups .. -1 to this as it mutates return type and overlaps with data already returned
15:35:38 <cyberpear> basically parsing useful details from the output of a command into a list of dicts for the findall case, or just a dict in the search case
15:35:59 <bcoca> if you want new 'regex_nice_dicts' filter, that is fine, but leaning to not altering existing this way
15:36:07 <felixfontein> how about doing this as a new filter?
15:36:13 <bcoca> jinx
15:36:16 <felixfontein> yes
15:37:02 <bcoca> sometimes there is good reason to have mutated returns .. i dont see this as one of those cases
15:37:07 <cyberpear> bcoca: how would you handle it? -- the regex_search already returns either a string or a list, based on the options requested, IIUC
15:37:18 <cyberpear> bcoca: would you be happier if it was always a list of dicts?
15:37:34 <bcoca> and you change the type from under existing filter users?
15:38:01 <cyberpear> bcoca: the type is only different if the "groupdict" option is selected; otherwise, the behavior is not changed
15:38:40 <bcoca> you just said 'always'
15:38:59 <bcoca> but still stands, i think this merrits new filter and not modifying existing
15:39:02 <cyberpear> I see. in the groupdict case, wrapit in a list
15:39:13 <bcoca> i know current implementation only does it on 'true'
15:39:33 <bcoca> but you already get group lists from findall (if im reading python docs right)
15:40:19 <cyberpear> yes, findall gives a list regardless
15:41:05 <felixfontein> but it will be list of dicts instead of list of lists?
15:41:27 <cyberpear> right, `regex_findall` with `groupdict` will return a list of dicts
15:41:51 <bcoca> even tempted to say .. create a filter that you use on the output of this filter
15:42:35 <cyberpear> not sure how that would be possible
15:42:51 <bcoca> |group2dicts ?
15:43:14 <cyberpear> right now, the names of the groups are lost
15:43:19 <felixfontein> I think that would be a lot worse than a new filter
15:43:21 <felixfontein> yes
15:46:57 <bcoca> ah, didnt realize names were lost .. still, new filter
15:48:01 <cyberpear> right now, regex_search returns a string unless you ask for groups, in which case it returns a list
15:48:08 <cyberpear> list of strings
15:48:22 <cyberpear> is a list of dicts so far different that it needs a new filter?
15:48:30 <cyberpear> and if so, what names?
15:48:49 <cyberpear> and generally, when is an option to an existing filter vs a new filter
15:49:00 <sdoran> It should probably always return a list.
15:49:16 <sdoran> That it sometimes returns a string I would consider a bug.
15:50:56 <sdoran> It's hard to say, but wanting to return a different data type is one case where a new filter should be used rather than using a toggle that changes the returned data type.
15:51:31 <bcoca> sdoran: it does `return re.findall()` so that goes to the source  .. and has been that way for the life of the fitler
15:51:45 <bcoca> so i would have 'fixed that' in original creation, but not at this point
15:51:51 <sdoran> Agreed.
15:52:13 <cyberpear> name for the new ones?
15:52:15 <bcoca> just cause it 'does it wrong' does not mean we push down the sam epath more
15:52:57 <bcoca> idk, i would have to sit down and look at a collection of regex functions and probalby create nicer and more consistenet ones .. but 'time' ...
15:53:05 <cyberpear> just _groupdict?
15:54:17 <cyberpear> what about passing dict2items?
15:54:33 <bcoca> going to punt on bikeshed about names, leaving that for future meeting
15:54:34 <cyberpear> then it's a list again. would that work?
15:54:59 <bcoca> no, i think we mostly agree, new filter
15:55:01 <cyberpear> s/work/be acceptable/
15:55:04 <bcoca> leave current one as is
15:55:18 <bcoca> it would be a list, but not 'same list'
15:55:27 <bcoca> you would be inserting new data that is not like existing data
15:55:52 <bcoca> its still pushign adict ... just listified .. which is almost worse
15:56:51 <cyberpear> ok, wil adjust copy paste with _groupdict suffix
15:57:27 <bcoca> i woudlnt as you would not have string/list mutated return also
15:57:33 <bcoca> s/not/now/
15:57:49 <cyberpear> it'll always return just the groupdict verison
15:57:58 <cyberpear> or raise an error if it can't?
15:58:38 <bcoca> no talking about the findall inconsistency
15:58:52 <bcoca> but i'll wait for implementaiton before i comment further
15:58:57 <bcoca> #topic open floor
15:59:08 <cyberpear> it'll be 2 filters. one returns dict, other list of dict
16:01:14 <bcoca> #endmeeting