15:48:44 <maxamillion> #startmeeting Ansible Core 2019-08-08
15:48:44 <zodbot> Meeting started Thu Aug  8 15:48:44 2019 UTC.
15:48:44 <zodbot> This meeting is logged and archived in a public location.
15:48:44 <zodbot> The chair is maxamillion. Information about MeetBot at http://wiki.debian.org/MeetBot.
15:48:44 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
15:48:44 <zodbot> The meeting name has been set to 'ansible_core_2019-08-08'
15:49:09 <maxamillion> #link Agenda: https://github.com/ansible/community/issues/482
15:49:35 <maxamillion> #topic felixfontein implemented "option b" as a follow up to last meeting
15:49:37 <felixfontein> I have to leave soon, so some quick comments on my questions:
15:49:56 <maxamillion> #info "option b" == https://github.com/ansible/ansible/pull/60178 in respect to https://meetbot.fedoraproject.org/ansible-meeting/2019-08-06/ansible_core_public_irc_meeting.2019-08-06-19.02.html
15:50:09 <felixfontein> Aa) I've recreated a follow-up PR for option b), plPTAL :)
15:50:34 <felixfontein> b) the module validation PR is I think uncontroversial, but I still wanted to ask whether the extra validations in there are OK
15:50:34 <shertel> We did not have a quorum for that vote. Does anyone else have an opinion? Jill and I both preferred b.
15:51:08 <felixfontein> (and sorry for my typing, was just traveling through a almost-no-internet area :) )
15:52:11 <maxamillion> felixfontein: I think it looks good but I'd consider this a major change in the changelog fragment and not a minor one, I could see this potentially breaking playbooks
15:52:41 <felixfontein> maxamillion: fine for me; so far I thought that major_changes are reserved for core (non-module) changes
15:52:42 <shertel> do we usually make major changes for deprecation?
15:52:45 <sdoran> major_change is for changes to Ansible itself, not usually modules.
15:52:48 <maxamillion> felixfontein: oh ok
15:52:55 <felixfontein> shertel: not that I know of
15:53:04 <felixfontein> maxamillion: but I was just assuming that, no idea whether it's true
15:53:05 <maxamillion> abadger1999: ping - mind adding a comment/context around that? ^^^^
15:53:25 <maxamillion> shertel: I honestly don't know, it just "feels major" :)
15:53:55 <felixfontein> afk, bbl in 10 minutes
15:54:12 <sdoran> https://docs.ansible.com/ansible/latest/community/development_process.html#creating-a-changelog-fragment
15:54:18 <shertel> Yeah, I could see that. I guess I thought not since it's just modules that will have a deprecation period before people need to worry about playbooks breaking.
15:54:34 <shertel> sdoran +1
15:54:51 <sdoran> I read that all the time because I forget. :)
15:55:37 <sdoran> I think b is the best transition option, though it is a bit messy under the hood.
15:56:01 <shertel> Yeah, it's a bit of a hack
15:56:03 <sdoran> But those using the modules will most likely think they are two different modules, so I think the behavior difference based on module name is ok.
15:56:19 <sdoran> And they behave like two different modules, so it's ok.
15:59:24 <abadger1999> What's the context of what you need from me?
16:00:26 <sdoran> Clarity on changelog fragment headers. But I think our docs explain it well.
16:00:51 <sdoran> Is there  a case where changes to modules would fall under `major_changes`?
16:01:07 <sdoran> In this case, changing the name and behavior of a several modules.
16:01:09 <maxamillion> also, as an idle curiosity off to the side ... do we have a document somewhere that defines what qualifies as an `ansible_fact` instead of info?
16:01:31 <abadger1999> I think maybe.... but would have to evaluate case-by-case until we can detect a pattern.
16:01:32 <shertel> (but changing them in such a way that is backwards compatible with a deprecation warning)
16:01:58 <abadger1999> (like, oh all the core file related modules have removed dest in favor of path.  That would definitely be a major change ;-)
16:02:31 <shertel> The thing that's changing is whether ansible_facts is returned
16:02:37 <abadger1999> What are the modules involved here?
16:03:01 <shertel> 114 of them - https://gist.github.com/felixfontein/a6a1be999698da0ab3bd8868de6f16e1
16:03:26 <shertel> or 113, he removed one
16:03:34 <sdoran> @maxamillion https://docs.ansible.com/ansible/latest/dev_guide/developing_modules_checklist.html#contributing-to-ansible-objective-requirements <= last bullet point there
16:03:44 <maxamillion> sdoran: perfect, thanks
16:05:42 <abadger1999> Okay, I think I personally would prefer major_change but would accept either major_change or minor_change.
16:06:09 <sdoran> abadger1999: Awesome. Thank you.
16:06:14 <abadger1999> It should also show up in deprecation and removed, right?  Which people should be treating as major changes in and of themselves.
16:06:15 <shertel> abadger1999: do you have an opinion on a) b) or c)?
16:06:47 <shertel> (a) Deprecate these _facts modules and create a new _info module for each of them
16:06:56 <shertel> (b) Rename them, and make them programatically return ansible_facts when called as xxx_facts, or not ansible_facts when called as xxx_info, with a deprecation warning when xxx_facts is called
16:06:59 <abadger1999> So I'm not too worried if it is a minor_change if it also shows up in deprecate/removed.
16:07:05 <shertel> (c) Rename them, make them return both ansible_facts and the same results outside ansible_facts, and warn in the documentation that the ansible_facts results will be deprecated and removed in 2.13 (there will be no warning when these facts will be used - that might change in 2.10, though)
16:07:26 <felixfontein> re
16:09:01 <felixfontein> maxamillion: about breaking playbooks: I hope that before the _facts modules will be removed, we'll have functionality to inform people when they use deprecated facts; so there will be a window where they can find out what to change more easily.
16:09:12 <felixfontein> (not the whole 4 version deprecation window, but at least a subset of it)
16:10:08 <abadger1999> b is kind of neat techically.  I can't see any ways in which it is too smart for its own good off the top of my head (ie: it subtly breaks).  However, one of the ways to move people off of deprecated things is to tell them during the deprecated period "This bug won't be fixed in xxx_facts.  Use xxx_info instead where the bug is fixed".)  So that social engineering aspect gets lost.
16:10:34 <maxamillion> felixfontein: fair
16:10:38 <abadger1999> Also, when you eventually remove the xxx_facts file altogether, you'll have to create the files at that time with the removal boilerplate.
16:11:08 <abadger1999> So you're not gaining much over (c).
16:11:49 <felixfontein> except that people can opt in to use the _info name, and then they know whether they're still relying on the facts somewhere because their playbooks will fail if they do
16:12:18 <felixfontein> so from usability, I think (b) (and also (a)) is better than (c)
16:12:56 <abadger1999> b might possibly affect our present tooling for detecting when a module deprecation has expired and it should be removed but someone would have to test that to see.
16:13:15 <shertel> good idea
16:14:09 <felixfontein> abadger1999: all these renames (113) should expire at the same time, so I'd assume we will notice and won't forget to do that manually :)
16:15:40 <abadger1999> @felixfontein internally, I don't think we'd remember unless the tooling catches it.  ;-)
16:15:57 <felixfontein> does the tooling handles renamed modules (with old name deprecated) correctly anyway?
16:16:30 <abadger1999> (We are notoriously bad about remembering to remove deprecated code but our tooling has gotten better at telling us when there's deprecated stuff to deal with)
16:17:02 <felixfontein> I guess we'll find out for Ansible 2.10, because at least one renamed module's old name will expire then
16:17:07 <abadger1999> (BTW, this is just analysis.... I don't think I see anything that blocks option b in here)
16:17:33 <abadger1999> <nod> I think we do.  I'll have to look at the releng docs to see what tests we enable and then disable to see.
16:18:11 <felixfontein> the tooling probably has to consider the git log when the alias was created, otherwise it won't know when to remove the deprecated alias
16:19:06 <abadger1999> I think the tooling for this would live in the ansible-deprecated-version pylint plugin (if it exists)
16:20:29 <abadger1999> felixfontein: Looks like it finds AnsibleModule.deprecate() calls but not the module renames
16:21:13 <felixfontein> ah. at least for the modules I renamed, there's a module.deprecate() call
16:21:25 <felixfontein> for the ones I didn't rename and wasn't involved in, IDK
16:22:06 <abadger1999> <nod>
16:23:16 <abadger1999> Anyway... I'm fine with (b) ;-)
16:23:25 <abadger1999> I can't see anything wrong with it.
16:26:45 <felixfontein> ok, so one more +1 for (b)?
16:26:59 <felixfontein> I have to leave again, thanks already everyone for looking at this!
16:27:02 <shertel> sdoran, maxamillion, have a vote for b?
16:27:09 <shertel> \o
16:27:15 <sdoran> +1 for option B
16:27:22 <sdoran> I reviewed the PR.
16:27:51 <shertel> Cool. I left a comment with abadger's suggestion about adding the deprecation notice.
16:28:02 <felixfontein> thanks sdoran, I'l ltake a look at your review later; I'll also ask acozine on the formulation once everyone else is happy with the technical side :)
16:28:07 <maxamillion> shertel: +1 for b
16:28:12 <maxamillion> sorry
16:28:13 <sdoran> felixfontein: Just some minor things.
16:28:17 <maxamillion> multitasking badly
16:28:39 <sdoran> That's the _only_ way to multitask.
16:29:44 <shertel> Updated the vote tally
16:30:10 <shertel> thanks for more input
16:41:39 <sdoran> Is our metting complete @maxamillion ?
16:41:52 <maxamillion> I'm good
16:41:58 <maxamillion> #topic Open Floor
16:42:09 <maxamillion> if nobody has anything in the next 60 seconds, I'm pulling the trigger
16:42:21 * cyberpear want to re-open a docs PR that needs core discussion
16:42:46 <cyberpear> https://github.com/ansible/ansible/pull/57318
16:43:30 <cyberpear> it's a trivial change that documents an existing option w/ no functionality change, but the docs meeting thought it required core discussion
16:43:43 <shertel> Yeah, it seems like the other is stagnate
16:44:18 <cyberpear> (I can't re-open it myself since it was closed by not-me, and it's been locked, so I can't even comment on it.)
16:44:29 <sdoran> There's a PR I just reviewed to add that.
16:44:30 <shertel> reopened
16:44:35 <sdoran> Let me dig up the link.
16:45:04 <sdoran> https://github.com/ansible/ansible/pull/60127
16:45:28 <sdoran> I'm in favor of documenting this.
16:45:51 <sdoran> I'm working on an alternative solution to this translation of group characters, but this should be a documented option until I get something working.
16:46:12 <cyberpear> (I keep bringing up this closed PR, but everyone gets hung up on the fact that there's another far-reaching PR that also includes this tiny docs change)
16:46:41 <shertel> I usually try to merge the older of two prs to be more fair. If cyberpear's looks okay I'd merge that rather than the one opened 2 days ago.
16:47:10 <shertel> The other PR has some valuable stuff
16:47:25 <shertel> but I don't think we should hold up documenting the ignore option since it isn't moving
16:48:10 <sdoran> We have had enough noise around this that it needs to be documented.
16:48:36 <shertel> I think we're all in agreement.
16:49:14 <sdoran> And I meant "noise" not to be taken negatively: feedback on the pain/frustrtation around this behavior is much appreciated.
16:50:09 <sdoran> So whichever passes tests first we'll merge. Probably your PR cyberpear since the other has an erroneous code change still.
16:50:22 <sdoran> We need to wrap up. Anything else?
16:53:14 <maxamillion> #endmeeting