ansible_core_irc_public_meeting
LOGS
19:02:33 <jillr> #startmeeting ansible core irc public meeting
19:02:33 <zodbot> Meeting started Tue Jan 28 19:02:33 2020 UTC.
19:02:33 <zodbot> This meeting is logged and archived in a public location.
19:02:33 <zodbot> The chair is jillr. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:02:33 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
19:02:33 <zodbot> The meeting name has been set to 'ansible_core_irc_public_meeting'
19:02:38 <jillr> hi felixfontein
19:02:48 <felixfontein> hi jillr!
19:03:25 <jillr> tremble: looks like both of your PRs on the agenda have been merged, I'm going to mark them complete unless you had anything else to discuss?
19:03:58 <jillr> ah, and someone beat me to that  :)
19:04:06 <felixfontein> sorry :)
19:04:11 <jillr> nw!
19:04:16 <jillr> #topic https://github.com/ansible/ansible/pull/64341
19:04:17 <tremble> Nope, nothing to add
19:04:30 <felixfontein> cyberpear:
19:04:31 <jillr> cyberpear: you're up
19:04:48 <jillr> thanks tremble
19:06:30 <jillr> at first pass I think the changes for the unique filter make sense
19:07:41 <felixfontein> I vaguely remember that there was already a discussion of this (or something similar?)
19:08:12 <jillr> oh, I may have missed that
19:09:01 <felixfontein> I'm really not sure anymore, maybe it was also a similar problem (with another filter)
19:09:49 <jillr> I dont see '64341' in the last couple of Thursday meeting logs, and unfortunately my bouncer lost some logs this month
19:10:40 <jillr> but I don't think cyberpear is available today, so we can pass for now
19:11:01 <jillr> #topic https://github.com/ansible/ansible/pull/66389
19:11:40 <felixfontein> that one fixes a problem for modules which use add_file_common_args=True
19:12:10 <felixfontein> that setting adds several options which aren't documented and come from FILE_COMMON_ARGUMENTS, and which were added there apparently to simplify some action plugins
19:12:43 * jillr reads
19:13:17 <felixfontein> most infos are in the issue I think
19:15:51 <felixfontein> the main problem is that this PR might break random modules / action plugins which relied on the extra entries being in FILE_COMMON_ARGUMENTS. I found and fixed a couple in the PR, but it's kind of impossible to guarantee that it won't happen
19:17:37 <shertel> felixfontein: have you added ci_complete to the end of a commit? Obviously won't catch things without tests, but that will at least run everything available.
19:18:05 <felixfontein> I think it *should* run all tests, since it modifies module_utils/basic.py
19:18:13 <shertel> and it could of course still break 3rd party stuff
19:18:19 <shertel> ok
19:18:52 <felixfontein> yep, that too
19:19:13 <felixfontein> though one could argue that it has already been broken, since it only worked due to some undocumented assumptions :)
19:22:47 <jillr> it would be nice to have a better handle of testing/implications but as long as we're running all the tests we have available, that may be the best that can be done
19:25:21 <jillr> shertel: what do you think?
19:26:58 <felixfontein> I think that if this problem gets fixed, now's the best time (which isn't already in the past ;) )
19:27:35 <jillr> :)
19:27:38 * bcoca powers down tardis
19:28:06 <shertel> I'm not opposed to the change. I wish ansibot had not automatically removed needs_triage, seems like it would have been worth discussing with a few more of us.
19:28:37 <jillr> do you want to hold onto it for the Thur meeting, in case more folks show up?
19:29:13 * shertel suffers from indecision
19:29:24 <shertel> bcoca: have an opinion on merging that one?
19:29:28 <jillr> there are 3 of us here now, bcoca do you see any issue with this one?
19:29:35 <jillr> jinx shertel :)
19:30:24 <bcoca> force all tests, then do it x5 more times
19:30:30 <bcoca> breaking file is breaking everything
19:30:57 <bcoca> have not had deep look into PR, dont think felixfontein is wrong, just hard to be right and not break stuff in file
19:31:52 <bcoca> felixfontein: you would be shocked how many times people consider it a regresssion when you fix a bug that changes/stops some unspecified behavoiur they relied on
19:31:53 * jillr comments on pr to that effect
19:32:27 <felixfontein> everyone passing a invalid option to a module which allows it because of that could of course complain :)
19:32:42 <felixfontein> though in most cases, the module probably newer knew about that option and ignored it anyway
19:33:11 <shertel> yes, just some corner cases probably
19:34:32 <felixfontein> for me personally, keeping these extra options is worse in the long run than breaking a few things now (which hopefully will be spotted before 2.10.0 is released)
19:35:02 <felixfontein> just think of someone adding `backup: yes` to some module only to find out that no backup was created (because the module didn't knew about the option)
19:36:04 <tremble> would it be worth marking them as deprecated.  If someone explicitly added the option then it would override the definition...
19:37:16 <felixfontein> that won't show problems with action modules using FILE_COMMON_ARGUMENTS
19:37:34 <tremble> ah ok
19:39:28 <jillr> #topic https://github.com/ansible/ansible/pull/52053
19:39:57 <jillr> dont think jtyr is online though, and it looks like sam already reviewed this one
19:42:01 <jillr> I don't have anything to add to his review
19:42:08 <bcoca> remimnder: find time to burn mount module
19:42:29 <shertel> :-)
19:42:41 <jillr> #topic open floor
19:47:29 <jillr> let's call it then, thanks everyone!
19:47:33 <jillr> #endmeeting