19:03:21 <shertel> #startmeeting ansible core public irc meeting
19:03:21 <zodbot> Meeting started Tue May 12 19:03:21 2020 UTC.
19:03:21 <zodbot> This meeting is logged and archived in a public location.
19:03:21 <zodbot> The chair is shertel. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:03:21 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
19:03:21 <zodbot> The meeting name has been set to 'ansible_core_public_irc_meeting'
19:03:37 <shertel> #chair sdoran cyberpear felixfontein tremble
19:03:37 <zodbot> Current chairs: cyberpear felixfontein sdoran shertel tremble
19:04:13 <shertel> Not sure if bcoca is around... he has a few things on the agenda
19:04:34 <Shrews> shertel: he was feeling ill, so probably not around
19:04:41 <shertel> #info agenda: https://github.com/ansible/community/issues/536
19:06:09 <shertel> I'm not sure if we can cover the first two without him, so skipping those
19:06:33 <shertel> (unless someone else knows about those?)
19:07:02 <shertel> #chair Shrews
19:07:02 <zodbot> Current chairs: Shrews cyberpear felixfontein sdoran shertel tremble
19:07:13 <shertel> #topic https://github.com/ansible/ansible/pull/66650#pullrequestreview-346901038
19:07:47 <cyberpear> I'm all for backing out the deprecation, but maybe best to wait for bcoca
19:08:09 <sdoran> I'm in favor of backing out this deprecation and not changing the default.
19:08:10 <shertel> On monday bcoca brought that up because 2.10 is approaching and we need to make a decision
19:08:31 <sdoran> I'm not a fan of this change and should have spoken up more about it before it was merged.
19:08:36 <shertel> sdoran: I agree
19:08:49 <shertel> that's also what bcoca said on Monday
19:08:52 <cyberpear> did it have the desired effect on reducing support cases?
19:08:59 <felixfontein> I'm also in favor of keeping the current default (and removing the deprecation warning)
19:09:18 <sdoran> I think it limits the user on their group names, which is not great.
19:09:26 <bcoca> cyberpear: that was never the reason, it was reducing suprise to new users , support cases are a reflection
19:09:28 <bcoca> sorry im late
19:09:35 <shertel> #chair bcoca
19:09:35 <zodbot> Current chairs: Shrews bcoca cyberpear felixfontein sdoran shertel tremble
19:09:50 <bcoca> sdoran: group names were always limited, just inconsistently
19:10:10 <felixfontein> sdoran: I adjusted all our group names > one year ago, and I still get find it ugly to have to use underscores (as opposed to dashes which I use for hostnames)
19:10:15 <bcoca> and i didnt like the solution either, but it was the one voted for, not what i wanted (but that would have gotten pushback too)
19:11:00 <shertel> yeah, the current behavior was several compromises after the initial attempt
19:11:01 <bcoca> felixfontein: if we did that for hostnames, we would hvae to mandate ansible_host be populated, since its not normally used as var, we decided to side step that (almost no reports of isssues on that part)
19:11:22 <felixfontein> bcoca: I'm not suggesting we should do it for hostnames too, I want to be more liberal for group names :)
19:11:38 <felixfontein> and I think if we do that for hostnames, users really want to kill us
19:12:03 <bcoca> they have to find me first!
19:12:26 <bcoca> kiddign aside, i was just explaingin the history of it, not advocating
19:14:22 <bcoca> so my vote is to just drop the deprecation message, leave things 'as is' .. not great default, but - fans will rejoic e
19:14:23 <shertel> It seems uncontroversial, but I guess we could vote. Does anyone want the default behavior to change?
19:14:28 <bcoca> or re-joice?
19:14:56 <bcoca> a) go ahead with deprecation + change default, b) remove deprecation keep default c) beer
19:15:09 <bcoca> a+1, b-1, c+10k
19:15:36 <shertel> wait, you're +1 for changing the default?
19:15:42 * shertel is confused
19:15:48 <cyberpear> so, anyone want to keep the deprecation? (do we have quorum?)
19:15:50 <bcoca> sorry, fever here
19:16:00 <shertel> oh, sorry
19:16:00 <bcoca> a= remove mesg, keep CURRENT default
19:16:08 <bcoca> no, thanks for catchign that
19:16:16 <bcoca> i might make less sense than usual today
19:16:31 <shertel> +1 for keeping current default behavior
19:16:32 <bcoca> monday bcoca said waht i wanted, if you guys remember, i might just leave that and go back to bed
19:17:09 <sdoran> +1 for keeping current default and removing message (I believe that's "a")
19:17:10 <shertel> feel better
19:17:34 <felixfontein> +1 for keeping current default and removing deprecation warning
19:17:37 <cyberpear> +1 keep current (legacy) default, remove deprecation
19:17:43 <sdoran> bcoca: Sorry you're feeling under the weather.
19:18:06 <shertel> Ok, so +5, none opposed.
19:18:34 <cyberpear> (this will leave the non-deprecation warning, saying "you have bad characters")
19:19:15 <bcoca> im fine with warning, easy to shut off, maybe change message to say 'problematic' and link to faq/docs
19:19:27 <cyberpear> 4 settings: ignore, never, always, silently -- never and always give warnings about the bad characters
19:19:31 <sdoran> Oh, I thought that's the warning we removing. Sorry.
19:19:37 <sdoran> Removing the _deprecation_ warning.
19:19:40 <sdoran> 🤔
19:19:40 <felixfontein> cyberpear: true, but you can turn it off
19:19:40 <bcoca> sdoran: yes, but diff warning
19:20:03 <bcoca> warning informs newbs and veterans know how to shut off
19:20:05 <cyberpear> 2 warnings -- currently can use "ignore" option to turn off both... status quo is continue to warn, but allow old behavior
19:20:49 <cyberpear> the accepted +5 "keep current default" drops the deprecation warning, but keeps the "you have bad chars" warning
19:21:24 <shertel> yes, I think the second warning should be addressed separately if we want to change that
19:21:28 <sdoran> I guess that's fine. Changing to a _different_ default is probably a bit much.
19:21:38 <sdoran> Yeah, let's just go with what we've decided for now.
19:21:48 <sdoran> Revisit when people get annoyed with the warning. :)
19:21:50 <cyberpear> it's a definite improvement, and I'd push it live right away... I might advocate for removing both warnings, but that should be a separate discussion to avoid clogging the "change the default" discusion
19:22:00 <shertel> sounds good
19:22:01 <sdoran> Yup.
19:22:31 * bcoca goes to hole under bed to see if pain goes away in dark
19:22:31 <cyberpear> probably worthwhile removing the deprecation warning also from the released branches in the next round of updates, once it's merged to devel
19:22:52 <shertel> Yeah, that would make sense
19:23:21 <shertel> #topic open floor
19:23:29 <shertel> anyone have anything else they want to discuss?
19:23:38 <felixfontein> bcoca: get well soon!
19:24:17 <felixfontein> I'm not sure whether this is the right venue (probably not), but collection build shouldn't change symlinks to copies IMO
19:25:06 <cyberpear> I seem to run into spurrious conditional_bare_vars warnings sometimes, but haven't narrowed it down to a simple reproducer yet
19:25:10 <shertel> hm... didn't know it did
19:25:30 <felixfontein> (main reason next to that it wastes space, is that it screws up sanity tests - especially when galaxy starts to run them on import)
19:25:31 <cyberpear> when I heard that, I though that seems broken
19:25:54 <felixfontein> it apparently was a design decision
19:25:55 <sdoran> felixfontein: I think we've talked about this before. We would need to ask @mattclay for the reasoning behind that.
19:26:04 <sdoran> I forget the exact details.
19:26:06 <bcoca> jborean93 did it
19:26:11 <bcoca> by design
19:26:12 <jborean93> IIRC it was done that way due to us not knowing whether tar on the controllers will be able to handle symlinks. Also simplifies the install step
19:26:33 <jborean93> if we want to change that we can but that's what was chosen at the time build/publish/install was being developed
19:26:34 <sdoran> There it is. :) Knew there was a reason.
19:26:56 <bcoca> only point, i would still change to files, when symlink points 'OUTSIDE" colleciton
19:27:01 <felixfontein> sdoran: I'm not saying it makes no sense, but it has some drawbacks that are more obvious now :)
19:27:05 <bcoca> im fine with preserving 'inside'
19:27:07 <jborean93> bcoca: right now we fail on that
19:27:26 <jborean93> maybe warn, can't 100% remember
19:27:28 <bcoca> ack, just saying if we change to preserve, it should not do so for outside ones
19:27:43 <felixfontein> yes, definitely
19:27:45 <bcoca> i would resolve otuside ones, warn or error, all ok with me
19:28:05 <jborean93> it's a warning https://github.com/ansible/ansible/blob/devel/lib/ansible/galaxy/collection.py#L893
19:28:08 <bcoca> +1 for preserve
19:28:13 <felixfontein> actually, what does `ansible-galaxy collection install` do when it encounters symlinks? (or symlinks to other directories, say to /etc/passwd?)
19:28:36 <jborean93> that's a good question
19:29:04 <felixfontein> might be a CVE lurking ;)
19:29:17 <bcoca> iirc, the 'install issue' was more about pip (when people still wanted to isntall via pip)
19:29:23 <jborean93> potentially, https://github.com/ansible/ansible/blob/devel/lib/ansible/galaxy/collection.py#L1129-L1157 is the code for extraction
19:29:28 <cyberpear> seems sane to reject symlinks outside of the src directory
19:29:47 <bcoca> yeah, isntall should never allow outside symlink
19:29:56 <felixfontein> if it doesn't break symlinks in general, that's kind of nice, since then we would only have to fix the ansible versions used to build collections, and not every one which installs collections
19:33:16 <felixfontein> it doesn't look like it handles symlinks
19:34:33 <jborean93> does it skip/convert or some other behaviour
19:36:19 <jborean93> also what does it break with sanity checks by not having a symlink?
19:36:26 <felixfontein> it will either create a directory of that name, or try to create a regular file with whatever contents
19:36:49 <felixfontein> jborean93: sanity tests verify that non-symlinked modules contain `module: <filename_without_py>` in them
19:37:15 <felixfontein> which will usually be wrong for symlinks (except if it simply points to another directory, but keeps the name)
19:37:19 <jborean93> I thought symlinked modules are going the way of the dodo in favour of routing?
19:37:23 <felixfontein> such symlinks are needed for 2.9 compatibility
19:37:33 <felixfontein> routing is only supported by 2.10+
19:38:09 <cyberpear> (for posterity)
19:38:11 <cyberpear> #agreed keep current TRANSFORM_INVALID_GROUP_CHARS default setting, remove deprecation warning, backport deprecation removal; leave "invalid group chars" (non-deprecation) warning  in place
19:39:05 <shertel> is it "#agree"?
19:39:10 <shertel> #agree keep current TRANSFORM_INVALID_GROUP_CHARS default setting, remove deprecation warning, backport deprecation removal; leave "invalid group chars" (non-deprecation) warning  in place
19:39:28 <felixfontein> the bot linked to http://wiki.debian.org/MeetBot
19:39:47 <felixfontein> that mentions `agreed`
19:39:57 <shertel> oh, thanks
19:40:03 <cyberpear> I think the bot gives no feedback. Just shows up in the minutes.
19:40:57 <shertel> Okay... if that's all, ending in a minute
19:41:50 <shertel> #endmeeting