19:04:11 <bcoca> #startmeeting ansible core public irc meeting
19:04:11 <zodbot> Meeting started Tue Jul 21 19:04:11 2020 UTC.
19:04:11 <zodbot> This meeting is logged and archived in a public location.
19:04:11 <zodbot> The chair is bcoca. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:04:11 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
19:04:11 <zodbot> The meeting name has been set to 'ansible_core_public_irc_meeting'
19:04:16 <sdoran> o/
19:04:30 <bcoca> #topic https://github.com/ansible/ansible/pull/70724
19:04:42 <bcoca> tchernomax?
19:04:47 <max_> yes
19:04:55 <bcoca> you brought it up
19:04:59 <bcoca> make your case
19:05:30 <max_> yes, I would like to know why exactly "we do not want state functionality in the template module"
19:05:43 <max_> and if I can make you change your mind
19:06:20 <bcoca> you can try, the main reason is that we don't think it belongs there nor in copy, thoese modules have a very specific purpose and removing files is not one of them, that is what the file module is for
19:06:49 <bcoca> otherwise why are not ALL modules that create/modify/manipulate a file providing this also?
19:07:36 <sdoran> We suffer from a number of "kitchen sink" modules that do too much and supporting them over time is problematic as they have become quite complex.
19:07:52 <sdoran> We prefer modules that do a small set of things very well and focus on that.
19:07:54 <bcoca> i thnk 'file' itself is one of those
19:08:00 <sdoran> True.
19:08:02 <Shrews> I think it muddies the purpose of the modules, IMO.
19:08:05 <agaffney> imo, it makes more sense in 'template' than it does in 'copy', but with there already being a way to do it with 'file' in a more general way, there's not a strong need for it in 'template'
19:08:14 <sdoran> `state: touch` comes to mind
19:09:17 <max_> the main reason for me to want a `state: absent` in template is to be able to simply "reverte" the task
19:09:39 <bcoca> but absent wont do that, it will delete teh file
19:09:51 <bcoca> if you want to 'revert' you need to know/keep previous state
19:09:56 <bcoca> that is normally done via backukp
19:10:52 <bcoca> each task only has the data pertaining to it, unless you feed it 'previous state' data, how is template going to know what that was?
19:11:09 <max_> generally you use template to create file , that's what I had in mind when I said "reverte"
19:11:09 <bcoca> you can do what you want with set of tasks/role, but not within a module
19:11:20 <bcoca> max_: no, in many cases you use template to update a file
19:11:29 <bcoca> i woudl argue. most cases it updates
19:11:40 <agaffney> I don't think they meant an actual revert, just the reverse of creating the file with a template (removing the file)
19:11:43 <bcoca> as it is mostly used to maintain existing config files
19:11:52 <max_> @agaffney yes
19:12:08 <bcoca> again, use file module for that, since it would not be a true 'revert' for template in any case
19:13:10 <max_> I though about that and using file I end up having *a lot* of `template/when: truc` `file/when: not truc`
19:13:27 <max_> which make the playbooks/role mode difficult to read
19:14:07 <sivel> The next thing people would complain about is not wanting to have to specify src with state=absent, it's an endless road that is better solved with current functionality
19:14:19 <max_> I agree that the copy module is hard, so adding a `state` parameter to it add even more complexity without much gain
19:14:34 <bcoca> max_: i would argue you need to restructure your playbook
19:14:41 <max_> I added it to the template module as it's still simple
19:15:06 <sivel> I don't think we have any core team vote in favor of adding this behavior
19:16:18 <max_> adding `state` in template is basically 10 lines in a if ; I think it's not very complex
19:16:36 <bcoca> its also very few lines adding it to lineinfile, copy and blockinfile
19:16:44 <bcoca> does not mean we SHOULD add it
19:16:57 <max_> yes
19:17:08 <sivel> Without anyone else in favor of this, I think we can move on.
19:17:37 <max_> doesn't meen ever you shouldn't; adding it to template doesn't mean you have to add it to copy, lineinfile and blockinfile
19:18:03 <bcoca> it has the same rationale, if someone writes a playbook that would be made easier by it
19:18:14 <bcoca> my point is that i dont think it belongs in template nor any of the others
19:18:17 <agaffney> slippery slope and all that
19:18:29 <max_> agreed
19:18:51 <bcoca> i can justify adding it if you show that it should be inherit to template, but nto cause a specific playbook was written in a way that would benefit from this
19:19:17 <bcoca> specially since that is a very niche need and not something that improves the module for general usage
19:19:59 <max_> ok
19:20:23 <bcoca> #topic open floor
19:21:35 <caleb15> Hi all, I'm wondering if anyone is interested in reviewing https://github.com/ansible/ansible/pull/43128. The corresponding issue is in the top 10 issues sorted by likes and it's not a large code change so I think it would be a easy win.
19:22:10 <bcoca> its on my list, but bigger question is do we really want to enable lack of until condition
19:23:24 <agaffney> I think it would be useful. I've seen lots of people in #ansible confused about why adding 'retries' by itself doesn't do anything, and there's a sane default that could be used (`until: registered_var is success`)
19:23:56 <caleb15> Same. I also had that problem
19:24:23 <agaffney> although, it looks like that PR would break things for people that currently only define 'until', since it removes the default for 'retries'
19:26:52 <sdoran> I think that would be a nice change to the interface.
19:26:58 <sdoran> Removing the default retries seems incorrect.
19:27:34 <agaffney> I assume the removal of the default was so that the retries would only trigger when explicitly specified, since the old code used the presence of 'until' to trigger retries
19:28:03 <agaffney> it would need to support both modes of operation, with the retries=3 default when 'until' is specified
19:28:15 <sdoran> Agreed.
19:28:30 <sdoran> Looks like it retries one time with this PR if I'm reading it right.
19:28:57 <caleb15> Uploaded file: https://uploads.kiwiirc.com/files/74a6a00852bd87b88d8f509be921344c/image.png
19:29:00 <caleb15> I could be wrong but it looks like it doesn't remove the default for retries. It sets it to 3 if there is a untill, 0 otherwise
19:29:35 <agaffney> I didn't dig too deep in the changed code. I just saw the default was removed from the attribute spec
19:29:53 <bcoca> on the other hand i had the intention of deprecating retries/until and folding it into current loop
19:30:05 <bcoca> right now they have weird behaviours since they are concentric loops
19:30:23 <bcoca> https://github.com/ansible/ansible/pull/62151
19:30:29 <agaffney> it does look like that would preserve the current behavior
19:31:02 <bcoca> need a lot more tests, sadly the current behaviour is not well tested/defined
19:32:19 <sdoran> It preservers the default in an odd way.
19:33:13 <agaffney> it does, but it's necessary with the way that it worked previously
19:36:11 <agaffney> even with bcoca's intention to deprecate this feature in favor of loop behaviors, I see no reason not to merge this (or something like it) in the short term
19:36:33 <agaffney> as it would be relatively non-disruptive and useful
19:36:41 <sdoran> Seems reasonable. I'm not opposed to the change, but the implementation needs more scrutiny and tests.
19:37:04 <bcoca> im -0 on the feature, but agree with sdoran on work needed
19:37:32 <cyberpear> The idea seems quite useful to me... otherwise, you waste register/until "resultt is not failed"
19:37:33 <sdoran> Well, it has tests. I just mean we need to make sure the existing tests have cover enough scenarios.
19:38:58 <bcoca> sdoran: yep, as i typed above, we lack tests and definition for current behaviours, which makes modifying it a bit scary
19:39:08 <sdoran> Exactly.
19:40:27 * bcoca was going to deprecate and create a well defined new facility to avoid having to cover existing
19:50:28 <bcoca> any more to add?
19:50:32 <caleb15> So does someone want to leave a review on the PR asking for more tests?
19:53:52 <bcoca> #endmeeting