19:00:34 <bcoca> #startmeeting ansible core irc public meeting
19:00:34 <zodbot> Meeting started Tue Jun 11 19:00:34 2019 UTC.
19:00:34 <zodbot> This meeting is logged and archived in a public location.
19:00:34 <zodbot> The chair is bcoca. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:00:34 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
19:00:34 <zodbot> The meeting name has been set to 'ansible_core_irc_public_meeting'
19:00:42 <sdoran> o/
19:00:45 <bcoca> #topic https://github.com/ansible/ansible/pull/57623
19:00:50 <bcoca> tchernomax ?
19:00:50 <jillr> o/
19:01:04 <tchernomax> bcoca yes
19:01:20 <bcoca> so we were discussing during triage and 3 of us had 5 opinions on this
19:01:30 <bcoca> the only thing we agreed is that the current implementation is not what we want
19:01:37 <tchernomax> ok
19:01:49 <nitzmahone> o/
19:01:53 <bcoca> so there was a) just reject b) allow but make it into new option c) allow but new filter
19:02:09 <bcoca> ^ if i forgot any please feel free to add proposals for this feature
19:02:19 <tchernomax> b or c are fine for me :)
19:02:45 <bcoca> it can also get more complicated as 'list merge' has 2 ways, a) unique elements b) allow duplicates
19:03:04 <tchernomax> yes indeed
19:03:14 <sivel> the sentiment was that currently we didn't want a multi typed argument. at least from a minimal review
19:03:42 <bcoca> that too, arguments should be ither string or bool, not 'maybe string, maybe bool'
19:03:58 <bcoca> and most of us wanted to keep recurse as a strict bool
19:04:03 <bcoca> hence options b and c
19:04:11 <tchernomax> ok keep retro compat
19:05:07 <tchernomax> for my part I am more in favor of c)
19:05:09 <bcoca> that is one part, wanted to bring up here so we could discuss pro/cons and give you definite  direction (our triage meeting doesnt get user feedback and we have limited time per issue)
19:05:10 <cyberpear> list_merge={replace,unique,append}
19:05:37 <cyberpear> with `list_merge=replace` being the current behavior
19:05:48 <bcoca> works4me
19:05:51 <tchernomax> cyperpear, good idea
19:06:13 <bcoca> sdoran: you prefered diff filter, want to weigh in why that is better?
19:06:22 <sdoran> Different filter. Option C.
19:06:23 <tchernomax> but what about recursive=False, list_merge=append
19:06:36 <bcoca> list_merge ONLY is used if recursive=true
19:06:48 <sdoran> I think a filter name is easier to remember than passing options to an existing filter.
19:06:56 <bcoca> or we make combine applyt to lists at top level?
19:07:00 <maxamillion> .hello2
19:07:01 <zodbot> maxamillion: maxamillion 'Adam Miller' <maxamillion@gmail.com>
19:07:22 * mattclay waves
19:07:22 <cyberpear> sdoran: `| combine_with_list_merge`?
19:07:31 <bcoca> deep_combine?
19:07:56 <cyberpear> ^better
19:07:58 <maxamillion> combine_with_feeling
19:08:06 <maxamillion> sorry ... not helpful
19:08:11 <tchernomax> it seem less intuitive than a simple different filter
19:08:14 <maxamillion> +1 to deep_combine
19:08:15 <bcoca> combine_to_the_max
19:08:34 <tchernomax> +1 to deep_combine
19:08:36 <maxamillion> it's pythonic-ish in naming so we at least have some realm of familiarity
19:08:56 <bcoca> i prefer the option, since it would dupe a lot of the code and 'combine' is not really dict/list specific
19:09:05 <tchernomax> + an option to select `list_merge={replace,unique,append}`
19:09:12 <bcoca> i would say its more 'list' than dict (combinatorials)
19:09:21 <sdoran> A separate filter also removes the need to handle arbitrary strings as options, which in turn make a filter more complex.
19:09:26 <sdoran> I like simple things. :)
19:09:34 <cyberpear> if going w/ `deep_combine`, I'd say deprecate the `recursive=` option on the existing `combine`
19:11:12 <tchernomax> cyberpear: i am not against deprecate `recursive=` on `combine`
19:11:59 <bcoca> sdoran: filters dont require 'option handling' outside of normal python functions
19:12:10 <bcoca> i.e  (recursive=False, list_merge='replace')
19:14:26 <bcoca> quick vote?
19:14:26 <sdoran> Right. I'm just saying I would prefer a separate filter with discrete behavior rather than an option to an existing filter that changes the behavior. Both from a UI perspective and code complexity.
19:15:24 <tchernomax> bcoca: I vote for a separate filter
19:15:59 <bcoca> so lets do a) sep filter, b) additional option to existing
19:16:04 <bcoca> b+1
19:16:18 <tchernomax> a+1
19:16:39 <sdoran> a+1
19:17:16 <maxamillion> grrrr ... I'm torn
19:17:26 <bcoca> +-0 is valid vote
19:17:27 <maxamillion> b+1
19:17:52 <maxamillion> I was leaning b, the more I think about it the more I'd rather document how to use existing filters than adding a billion filters
19:18:01 <bcoca> k so 2/2 at this ponit
19:18:04 <cyberpear> b+1
19:18:14 <bcoca> c) outright reject the option, but i dont think anyone is strong on that one
19:18:19 <bcoca> 2/3/0
19:18:33 <bcoca> anyone else? closing vote in 1min
19:19:00 <jillr> I had been leaning a, but I think maxamillion may have swayed me to +0
19:19:00 <sivel> a+1
19:19:16 <bcoca> 3/3/0
19:20:13 <bcoca> k, closed at undecided 3/3
19:20:22 <bcoca> open to new arguments either way
19:20:31 <maxamillion> jillr: I originally was leaning A but I was like "wait, we have a thing for that already ... we don't need new code, we just need docs!" (granted I hold the right to be incorrect *and* nobody's ever going to read those docs ... but that was my personal "aha!" moment :) )
19:20:54 <bcoca> i prefer a couple of options on a filter than brand new filter that does almost same thing
19:21:19 <bcoca> ^ i find it simpler to haave 1 copy of code + 2 `ifs` vs 2 very smiilar copies of code
19:21:20 <jillr> maxamillion: seeing an implementation of B might get me to +1, so I'd def look at it again
19:21:46 <maxamillion> jillr: fair point
19:21:56 <tchernomax> with b) i don't like the fact the option list_merge will only be used when recursive=True
19:22:54 <bcoca> tchernomax:  in the other filter, its mostly the same, it wont be used outside recursion
19:23:03 <shertel> a+1
19:23:04 <bcoca> just not explicit, since it is implied
19:23:12 <tchernomax> bcoca yes indeed
19:23:37 <shertel> but in favor of sharing code rather than duplicating
19:23:39 <maxamillion> tchernomax: oh, that's a good point
19:24:39 <bcoca> k, let me propose we revisit this on thursday, give time for people to mull over what was said so we can make firm decision
19:24:55 <bcoca> and come up with other arguments to sway the rest
19:25:41 <tchernomax> bcoca: ok
19:27:10 <bcoca> #topic https://github.com/ansible/ansible/pull/57039
19:27:18 <bcoca> dominikholler?
19:27:25 <dholler> hi
19:27:27 <dholler> The problem is that this PR would change the behavior of ansible 2.8. The change in the behavior would affact only ports on OpenStack Networking API. The current behavior of ansible 2.8 works with neutron, but not with at least two other implementaions of the OpenStack Network API. The changed behavior would be more similar to ansible 2.7 and according to the OpenStack Networking API spec.
19:27:59 <bcoca> i would discuss that with openstack people, most of core stays out of that
19:28:34 <bcoca> emonty already approved, he has ability to merge
19:28:35 <cyberpear> unfortunately, thereos no "openstack people" who are responsive to anything related to ansible modules
19:28:48 <cyberpear> (that I've seen)
19:29:01 <cyberpear> #openstack-ansible only discusses the ansible playbooks for deploying openstack
19:29:05 <bcoca> cyberpear: check cc on any openstack module, they are the ones that wrote most of em
19:29:21 <dholler> this is the reason why the fix was not merged before the release of ansible 2.8
19:29:26 <bcoca> #ansible-devel is better place to contact 'ansible devs for openstack '
19:30:01 <bcoca> only discussion i see is tha tyou missed 2.8 .. nothing about not merging current
19:30:19 <bcoca> ah, this is backport PR of a feature?
19:30:32 <dholler> yes, it is just about the backport of a bugfix
19:31:00 <dholler> already merged in devel
19:31:01 <bcoca> @abadger1999 is already involved, he is the RM, you mostly need to discuss with him
19:31:13 <dholler> he directed me to this meeting
19:31:40 <abadger1999> Yeah,  is the one that might be a feature?
19:32:05 <dholler> no, it is just a plain bugfix
19:32:07 <abadger1999> Ah change in behaviour.
19:32:23 <jillr> it's a behaviour change, but the behaviour is to unbreak a bug right?
19:32:29 <dholler> yes
19:32:51 <jillr> I'm +1 on this
19:33:08 <abadger1999> I feel I can block things on things that violate our rules but if there's an exception to be mad, that needs to be made by more than just me.
19:33:17 <abadger1999> So, that's why I had dholler bring it here.
19:33:32 <bcoca> k, that makes more sense now
19:33:42 <shertel> I'm also +1
19:33:59 <bcoca> ... i would argue that the code has issues, but most were preexisting ...
19:34:16 <bcoca> if it fixes a current issue that makes os_port unusable .. +1
19:34:36 <abadger1999> Since it is only present in 2.8.0 and 2.8.1 and it's a community module, I think that the behaviour change is okay.
19:34:43 <bcoca> i do think its a 'feature' ... but sometimes that is only way to 'fix a bug'
19:34:49 <cyberpear> +1
19:34:56 <tchernomax> +1
19:35:21 <bcoca> well, looks overwhemlingly in favor
19:35:37 <bcoca> i say it passes
19:35:43 <bcoca> #topic open floor
19:35:47 <bcoca> Announcement: We will continue testing the two latest versions of Fedora in our CI test matrix as we have been in the past. Our current test matrix runs the newer Fedora release with Python 3 and the older with Python 2. Going forward, both of those versions will be running Python 3.
19:36:05 <bcoca> ^ if anyone has any comments/feedback, we are open to hearing it
19:37:42 <bcoca> if no new business, ending meeting in 3 mins
19:43:20 <bcoca> #endmeeting