19:04:11 #startmeeting ansible core public irc meeting 19:04:11 Meeting started Tue Jul 21 19:04:11 2020 UTC. 19:04:11 This meeting is logged and archived in a public location. 19:04:11 The chair is bcoca. Information about MeetBot at http://wiki.debian.org/MeetBot. 19:04:11 Useful Commands: #action #agreed #halp #info #idea #link #topic. 19:04:11 The meeting name has been set to 'ansible_core_public_irc_meeting' 19:04:16 o/ 19:04:30 #topic https://github.com/ansible/ansible/pull/70724 19:04:42 tchernomax? 19:04:47 yes 19:04:55 you brought it up 19:04:59 make your case 19:05:30 yes, I would like to know why exactly "we do not want state functionality in the template module" 19:05:43 and if I can make you change your mind 19:06:20 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 otherwise why are not ALL modules that create/modify/manipulate a file providing this also? 19:07:36 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 We prefer modules that do a small set of things very well and focus on that. 19:07:54 i thnk 'file' itself is one of those 19:08:00 True. 19:08:02 I think it muddies the purpose of the modules, IMO. 19:08:05 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 `state: touch` comes to mind 19:09:17 the main reason for me to want a `state: absent` in template is to be able to simply "reverte" the task 19:09:39 but absent wont do that, it will delete teh file 19:09:51 if you want to 'revert' you need to know/keep previous state 19:09:56 that is normally done via backukp 19:10:52 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 generally you use template to create file , that's what I had in mind when I said "reverte" 19:11:09 you can do what you want with set of tasks/role, but not within a module 19:11:20 max_: no, in many cases you use template to update a file 19:11:29 i woudl argue. most cases it updates 19:11:40 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 as it is mostly used to maintain existing config files 19:11:52 @agaffney yes 19:12:08 again, use file module for that, since it would not be a true 'revert' for template in any case 19:13:10 I though about that and using file I end up having *a lot* of `template/when: truc` `file/when: not truc` 19:13:27 which make the playbooks/role mode difficult to read 19:14:07 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 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 max_: i would argue you need to restructure your playbook 19:14:41 I added it to the template module as it's still simple 19:15:06 I don't think we have any core team vote in favor of adding this behavior 19:16:18 adding `state` in template is basically 10 lines in a if ; I think it's not very complex 19:16:36 its also very few lines adding it to lineinfile, copy and blockinfile 19:16:44 does not mean we SHOULD add it 19:16:57 yes 19:17:08 Without anyone else in favor of this, I think we can move on. 19:17:37 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 it has the same rationale, if someone writes a playbook that would be made easier by it 19:18:14 my point is that i dont think it belongs in template nor any of the others 19:18:17 slippery slope and all that 19:18:29 agreed 19:18:51 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 specially since that is a very niche need and not something that improves the module for general usage 19:19:59 ok 19:20:23 #topic open floor 19:21:35 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 its on my list, but bigger question is do we really want to enable lack of until condition 19:23:24 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 Same. I also had that problem 19:24:23 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 I think that would be a nice change to the interface. 19:26:58 Removing the default retries seems incorrect. 19:27:34 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 it would need to support both modes of operation, with the retries=3 default when 'until' is specified 19:28:15 Agreed. 19:28:30 Looks like it retries one time with this PR if I'm reading it right. 19:28:57 Uploaded file: https://uploads.kiwiirc.com/files/74a6a00852bd87b88d8f509be921344c/image.png 19:29:00 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 I didn't dig too deep in the changed code. I just saw the default was removed from the attribute spec 19:29:53 on the other hand i had the intention of deprecating retries/until and folding it into current loop 19:30:05 right now they have weird behaviours since they are concentric loops 19:30:23 https://github.com/ansible/ansible/pull/62151 19:30:29 it does look like that would preserve the current behavior 19:31:02 need a lot more tests, sadly the current behaviour is not well tested/defined 19:32:19 It preservers the default in an odd way. 19:33:13 it does, but it's necessary with the way that it worked previously 19:36:11 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 as it would be relatively non-disruptive and useful 19:36:41 Seems reasonable. I'm not opposed to the change, but the implementation needs more scrutiny and tests. 19:37:04 im -0 on the feature, but agree with sdoran on work needed 19:37:32 The idea seems quite useful to me... otherwise, you waste register/until "resultt is not failed" 19:37:33 Well, it has tests. I just mean we need to make sure the existing tests have cover enough scenarios. 19:38:58 sdoran: yep, as i typed above, we lack tests and definition for current behaviours, which makes modifying it a bit scary 19:39:08 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 any more to add? 19:50:32 So does someone want to leave a review on the PR asking for more tests? 19:53:52 #endmeeting