19:05:04 <bcoca> #startmeeting ansible plublic core irc meeting
19:05:04 <zodbot> Meeting started Tue Apr  2 19:05:04 2019 UTC.
19:05:04 <zodbot> This meeting is logged and archived in a public location.
19:05:04 <zodbot> The chair is bcoca. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:05:04 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
19:05:04 <zodbot> The meeting name has been set to 'ansible_plublic_core_irc_meeting'
19:05:13 <maxamillion> .hello2
19:05:14 <zodbot> maxamillion: maxamillion 'Adam Miller' <maxamillion@gmail.com>
19:05:30 <nitzmahone> Most of the rest of the world just did their DST switch
19:05:32 <jillr> o/
19:05:35 <bcoca> #topic  Allow exception to azure_*_facts modules new in 2.8 that don't return facts (schedule for rename/deprecation in 2.9) @nitzmahone
19:05:45 <nitzmahone> So yeah
19:06:16 <nitzmahone> There was some confusion about how much of the `_facts` must return `ansible_facts` and otherwise should be named `_info` modules should apply to 2.8
19:06:19 <bcoca> -1 since that will hit deprecation cycle in 2.9 and we can avoid issue with all the new _facts modules added in 2.8 by renaming now
19:06:54 <nitzmahone> Since that rule agreement was quite late in the 2.8 cycle, I'm basically proposing that adherence in 2.8 be left to the maintainers of the modules in question.
19:06:57 <sdoran> o/
19:07:09 <nitzmahone> If they want to change it in 2.8, go ahead, but we shouldn't require it until 2.9.
19:07:19 <bcoca> i agree applying _info in 2.8 is a bit late in game, but still against them being named _facts (which was older rule though not documented/well enforced)
19:07:43 <nitzmahone> Basically it was a guideline (oft-violated) until agreed upon March 5
19:08:11 <agaffney> I'd say that any modules that are new in 2.8 should be renamed for 2.8. I don't know if that applies to azure_*_facts
19:08:46 <nitzmahone> There's already there's a ton of other Azure modules already named that way
19:09:07 <nitzmahone> So I just wanted to keep internal consistency within those modules for 2.8, and 2.9 will become consistent with both rules
19:09:09 <bcoca> but not all of those violate the rule, some do return ansbile_facts
19:09:16 <bcoca> so i argue they are already inconsistent
19:09:17 <nitzmahone> (old ones, pre 2.7)
19:09:17 <zikalino8265> in general i agree with that. i actually wanted to rename all _facts to _info in 2.8. but there's inconsistency problem 50% of _facts, 50% of _info
19:09:39 <bcoca> FYI, no one is arguing we change 'releaed modules', this discussion is just for 'new in 2.8'
19:10:22 <bcoca> we do plan to enforce the rule for ALL existing modules in 2.9, but that requires deprecation cycle
19:10:53 <bcoca> my view is we avoid that for all 'new in 2.8' modules since they are a big number in those violating the rule
19:10:56 <nitzmahone> Yeah, the mechanics of the rename/deprecation still need to be worked out, which is why I was going for "leave it up to the maintainers" for 2.8
19:11:32 <nitzmahone> One-off modules that aren't part of something where there's already a pattern in place, sure, go ahead and call it`_info` now
19:14:59 <bcoca> my point is not as much 'call it _info' as 'dont call them _facts'
19:15:22 <nitzmahone> Sure, but calling it `_blarg` is the same problem
19:15:27 <bcoca> but you'll have to call them _info in 2.9 anyways ...
19:15:54 <bcoca> agreed, just not the rule i think we should enforce (we all agreed for it to start in 2.9)
19:16:00 <maxamillion> yeah, I'm not a big fan of things named `_facts` that don't return facts
19:16:03 <nitzmahone> Anyway, the confusion seemed to stem from when those rules would be enforced; `_info` for non-facts was agreed for 2.9, but no timeline was mentioned for `_facts`
19:16:23 <bcoca> cause _facts was assumed to be preexisting (if not well documented nor enforced)
19:16:26 <nitzmahone> (and the recent agreement was right before the original freeze was supposed to occur)
19:16:53 <sivel> Also, see https://github.com/ansible/ansible/pull/54349 for the PR that will hopefully help enforce this in the future
19:17:24 <bcoca> +1 to have it in validate-modules, but i think part of the problem is that we depend on it too much, not every rule is enforcable via CI
19:17:42 <bcoca> since these rules were not in CI .. we have not enforced them well on review
19:18:05 <maxamillion> nitzmahone: recent agreement? are there meetbot logs I can read for context?
19:18:11 <nitzmahone> yes, jas
19:18:20 <sivel> that is something I have to help figure out. I'm semi tasked with trying to get more things we would look for in a manual review, automated in CI
19:18:44 <nitzmahone> https://meetbot.fedoraproject.org/ansible-meeting/2019-03-05/ansible_core_irc_public_meeting.2019-03-05-19.01.log.html
19:18:47 <nitzmahone> --^ _facts
19:18:53 <nitzmahone> https://meetbot.fedoraproject.org/ansible-meeting/2019-03-19/ansible_core_irc_public_meeting.2019-03-19-19.05.log.html
19:18:57 <nitzmahone> --^ _info
19:18:58 <maxamillion> nitzmahone: thanks
19:19:18 <bcoca> @sivel not discounting your efforts, just saying we should not expect all of those being caught by CI, some are hard, some are impossible (until we have AI .. but they we'll be busy eveding hunter/killers)
19:20:16 <bcoca> https://github.com/ansible/proposals/issues/56 <= older agreed standarize usage of _facts
19:20:19 <nitzmahone> By deferring "hard enforcement" to 2.9 (as with _info), I'm mainly trying to ease the scramble to change this last-minute as well as keep internal consistency
19:20:48 <nitzmahone> (56 is also still an open proposal, so has not been accepted/agreed)
19:21:24 <bcoca> parts of it had been agreed, the _info and 'rename setup' are mostly divrgent from main issue
19:21:27 <nitzmahone> Though with the two prior agreements on this, should probably be updated + closed
19:21:46 <bcoca> yep, i need to make time to review/close/update many proposals
19:22:27 <bcoca> https://meetbot.fedoraproject.org/ansible-meeting/2017-03-07/core_team_meeting.2017-03-07-19.11.html
19:22:34 <nitzmahone> Anyway, basically, the question is: are we forcing maintainers to retroactively change stuff that's been merged/in-flight for 2.8, or not? I'm voting for "not" since the rule came so late in the 2.8 cycle and we didn't explicitly discuss a timeframe.
19:23:06 <bcoca> im saying 'yes' cause the rule was preexisting .. though i admit not well documented/enforced
19:23:27 <zikalino8265> we mostly have problem with consistency. in general we would like to have everything either _facts or _info in 2.8, not half-way....
19:23:35 <nitzmahone> (which to me means "was not a rule" previously)
19:24:11 <maxamillion> I'm on the fence, it's not a great situation any way we chop it up
19:24:27 <nitzmahone> (to clarify zikalino8265's position, he's talking about within the Azure modules, rather than having a handful of _info's with a bunch of _facts)
19:24:28 <bcoca> agreed, that is why its here for a vote
19:24:44 <nitzmahone> Leaving up to maintainers is the humane choice ;)
19:24:51 <bcoca> zikalino8265: but you have modules that DO return ansible_facts
19:24:52 <nitzmahone> 2.9 is where we'll enforce
19:24:56 <bcoca> the distinction is based on that
19:25:18 <zikalino8265> they shouldn't return ansible_facts really
19:25:47 <bcoca> i would argue that if a module is doing something it shouldnt .. we should not ship it
19:26:04 <zikalino8265> these are the oldest ones
19:26:37 <bcoca> im only discussiong modules 'new in 2.8' older ones we already all agree will have to go though rename/deprecation cycle and we'll dot hat work in Ansible 2.9
19:26:39 <zikalino8265> and then at some point we learned that azure modules shouldn't return ansible_facts
19:27:14 <bcoca> anything we 'shipped' has to go through deprecation cycle, here the discussino is 'do we pile on the problem' or 'start solving the bulk of it before it hits release'
19:27:56 <nitzmahone> The Azure modules will have to do a whole pile anyway, so a couple more is NBD
19:28:02 <zikalino8265> i know. i was seeing this (renaming to _info) as an opportunity to fix all of these problems and have everything consistent
19:28:10 <nitzmahone> (and allowing consistency going forward)
19:28:13 <bcoca> i do appreciate your reasons for not wanting teh change, also nitzmahone's point, i just value avoiding increasing the problem at this stage above those concerns (not dismissing them, just diff valuation)
19:28:14 <zikalino8265> just not enough time to do it for 2.8
19:28:48 <bcoca> but we had renamed most of them, its only last week when you started 'rerenaming'
19:29:08 <bcoca> and i understand its late before freeze, but even after freeze i still see it as a time to 'fix this bug'
19:29:20 <bcoca> that is what the freeze is for
19:29:29 <bcoca> avoid new features, BUT allow fixing bugs
19:29:44 <zikalino8265> i know... that's because i am blamed for inconsistency now....
19:29:59 <bcoca> not blamging you, blame on us for not catching this sooner
19:30:10 <bcoca> im not interested in blame, just in solution going forward
19:30:27 <bcoca> we all know how this happend, plenty of blame is on our side for lack of docs/lax review
19:30:35 <bcoca> lack of clarity
19:30:51 <nitzmahone> Again, it comes back to the misunderstanding about when the `_facts` rule should be applied; I think we have plenty of background, so should probably just put to a vote
19:31:14 <nitzmahone> A +1 is "Apply in 2.9, leave to maintainers for 2.8"
19:31:16 <bcoca> -1
19:31:19 <nitzmahone> +1
19:31:20 <zikalino8265> hehe, didn't mean here. by other people in my team :-)
19:31:43 <bcoca> zikalino8265: sorry to hear that, but you can paste the above 'confession' if it helps you at all
19:31:59 <bcoca> ^ anyone else? more thoughts, votes?
19:32:28 <nitzmahone> zikalino8265: yeah, this is totally an internal communication issue, y'all just got caught in the middle of the misunderstanding
19:33:04 <bcoca> and for that i appologise, this should have been well documented much sooner and caught long ago
19:33:22 * nitzmahone ditto
19:33:45 <zikalino8265> so myself i would like to either continue and deprecate / rename all before freeze, or just keep everything as _facts
19:33:45 <bcoca> @here votes? thoughts? dont leave @nitzmahone and I in a tie!!!!
19:33:55 <cyberpear> -1 (<<vote doesn't count)
19:34:02 <nitzmahone> maxamillion, sivel, sdoran, agaffney, jillr, anyone else?
19:34:09 <bcoca> zikalino8265: so you are -1?
19:34:13 <sdoran> +1
19:34:15 <sivel> I don't understand what we are voting on specifically
19:34:32 <bcoca> +1 == keep 'new bad _facts' module and punt to 2.9
19:34:36 <bcoca> -1 == rename em now
19:34:46 <Xaroth> new as in, introduced in 2.8?
19:34:50 <bcoca> yep
19:34:50 <nitzmahone> (allow maintainers to decide for 2.8)
19:34:54 <Xaroth> -1
19:34:55 <sivel> tbh, I am -1
19:35:02 <sivel> maybe more like -0.5
19:35:26 <nitzmahone> So we'll have like 4 random Azure `_info` modules in 2.8
19:35:28 <zikalino8265> yes, i think i have to be -1 now, not enough time to make everything consistent
19:35:28 <bcoca> 3/-3.5
19:35:39 <nitzmahone> to go with like 20+ `_facts` modules
19:35:48 <jillr> +1 punt to 2.9 for existing modules
19:35:51 <bcoca> zikalino8265: wait, you want to rename the ones you can to _info ?
19:35:53 <zikalino8265> nitzmahone, there's around 30
19:35:55 <bcoca> <= confused
19:36:10 <cyberpear> I assume we can add a '_info' alias to all the existing _facts modules for 2.8?
19:36:14 <bcoca> nitzmahone: most _facts modules are new in 2.8
19:36:25 <zikalino8265> @bcoca inititailly that was my plan
19:36:29 <nitzmahone> "Renaming all" isn't one of the choices, it's "rename new for 2.8" or "allow them to stay `_facts` for 2.8"
19:36:29 <bcoca> cyberpear: that is what we were doing
19:36:49 <nitzmahone> cyberpear: not for 2.8, for 2.9
19:36:50 <bcoca> ok, lets start vote again, cause seems like we are confusing issues
19:37:09 <zikalino8265> (i meant deprecate/new name pattern for older of course)
19:37:18 <nitzmahone> If we force the rename in 2.8, it will be inconsistent
19:37:19 <bcoca> if you are OK with 'keep misnamed _facts modules added in 2.8' vote  +1
19:37:27 <bcoca> if you want them renamed before release , -1
19:37:34 <maxamillion> nitzmahone: sorry, got pulled into another meeting
19:37:35 <nitzmahone> Push poll much?
19:39:11 <nitzmahone> We're voting for "apply late-breaking rules around `_facts` module naming in 2.8" or not
19:39:43 <zikalino8265> i am +1
19:39:43 <bcoca> not 'in 2.8' but to NEW MODULES introduced in 2.8
19:39:48 <nitzmahone> The `_info` side of it is not required until 2.9, so I'm arguing that the `_facts` side that was agreed upon also very recently should also not be required until 2.9
19:39:51 <bcoca> aka 'modules that have not shipped as _facts' yet
19:40:10 <Xaroth> -1 for new modules, +1 for existing modules
19:40:13 <nitzmahone> (but that are part of a set of other modules that are similarly named, eg, azure_)
19:40:49 <bcoca> Xaroth: define new vs existing?
19:41:10 <sivel> fwiw, I didn't take our _facts/_info decision to be applied starting with 2.9, I took it to mean "immediately"
19:41:12 <Xaroth> new as in, new to 2.8, existing as in, existed prior to 2.8
19:41:53 <bcoca> ok, so we are at +5/-4.5 as of now
19:42:12 <sivel> And for clarification, I only said 0.5 instead of 1, just because I don't want to take up more time.
19:42:12 <bcoca> if no new input/votes, going to declare 'punt till 2.9' due to 'too close to call'
19:42:22 <sivel> My pref is -1, but I want a result
19:42:44 <sivel> so I think, we just go with our vote as is
19:42:46 <Xaroth> I'm with sivel on this.
19:42:49 <cyberpear> -1 for modules introduced in 2.8
19:43:15 <sivel> I don't think everyone is clear on what +1 and -1 mean.  So let's just leave it with +5 and move on with life
19:43:27 <bcoca> to clarify:
19:43:44 <bcoca> +1 == we allow the modules added in 2.8 devel to stay improperly named _facts
19:43:56 <bcoca> -1 == we rename them now to avoid _facts
19:44:09 <nitzmahone> (and introduce inconsistency)
19:44:17 <bcoca> inconsistency already exists either way
19:44:20 <nitzmahone> within existing Ansible ecosystems
19:44:23 <Xaroth> out of curiosity, what is the time/effort impact?
19:44:25 <jborean93> +1 it's not a big deal to add an alias in the future. It seems to bring more pain to Azure users avoiding its "consistent" way right now
19:44:38 <nitzmahone> --^ exactly
19:44:42 <bcoca> Xaroth: PRs to do both were done and merged last week, takes <1h
19:45:01 <nitzmahone> We're adding 30ish aliases for Azure modules in 2.9, a few more is "who cares"
19:45:02 <jborean93> plus this was only agreed really late in the cycle
19:45:18 <bcoca> no, _info was late, _facts was existing
19:45:24 <sivel> we have 15 minutes left in this meeting, and we've used 45 just for this.
19:45:27 <jborean93> was never enforced or followed
19:45:27 <nitzmahone> yeah, it's more about the consistency at this point than the effort
19:45:42 <jborean93> but agreed just finalise and get to the point
19:45:43 <bcoca> jborean93: not well enforced, but we have enforced in past
19:45:47 <nitzmahone> (in the amount of time we've dithered on this, we could've fixed/unfixed another 20 times)
19:45:54 <bcoca> agreed
19:45:58 <sivel> I believe we came to +5/-4.5
19:46:00 <nitzmahone> OK, moving on. We're leaving it
19:46:02 <sivel> +5 is winner
19:46:05 <bcoca> +6/-4.5
19:46:06 <Xaroth> I +1 the motion to move on.
19:46:13 <bcoca> ^ borean new vote
19:46:22 <bcoca> #topic open floor
19:46:24 <sivel> we'll start enforcing in 2.9 via CI, or close to it
19:46:29 <bcoca> 'THE YAYS HAVE IT'
19:47:05 <Xaroth> Open floor already? well, here goes
19:47:19 <Xaroth> https://github.com/ansible/ansible/pull/46687 could love some input
19:47:54 <Xaroth> changes from the core meeting I missed 2 weeks have been implemented, CI system is running as it was being wonky earlier today (<3 again, sivel )
19:48:12 <bcoca> ^ tis on my list .. but i has long list
19:49:02 <Xaroth> I know, but I was hoping for some other eyes as well :)
19:49:23 <Xaroth> especially it being a relatively small change
19:50:14 <bcoca> small change, but adding new magic vars always ends up having deep implications
19:50:38 <Xaroth> So I've noticed :)
19:52:31 <Xaroth> if anybody else has something, I'd gladly yield the floor to them.
19:52:33 <sdoran> It looks like most things have been addressed.
19:52:42 <sdoran> I can test it out tomorrow.
19:52:49 <Xaroth> Much appreciated, sdoran.
19:53:20 <bcoca> yeah, code looks good, i just wanted to make sure its the 'right variables' and try corner cases with diff ways of importing roles (i.e depenendiciues/roles/include/import)
19:53:39 * Xaroth nods
19:55:01 <bcoca> #endmeeting