10:01:37 <gundalow> #startmeeting Ansible PR review day
10:01:37 <zodbot> Meeting started Thu Sep 19 10:01:37 2019 UTC.
10:01:37 <zodbot> This meeting is logged and archived in a public location.
10:01:37 <zodbot> The chair is gundalow. Information about MeetBot at http://wiki.debian.org/MeetBot.
10:01:37 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
10:01:37 <zodbot> The meeting name has been set to 'ansible_pr_review_day'
10:01:43 <gundalow> #chair tremble
10:01:43 <zodbot> Current chairs: gundalow tremble
10:02:06 <gundalow> tremble: Shall we start with the PRs you mentioned?
10:02:45 <tremble> Can do
10:03:35 <felixfontein> I will be around probably only during the evening
10:04:01 <gundalow> felixfontein: ace, thank you :)
10:04:24 <tremble> I wasn't around for the last day, so I'm not sure how you want to run this...
10:04:39 <gundalow> tremble: it's very informal
10:04:45 <gundalow> #topic Test PRs
10:04:59 <gundalow> https://github.com/ansible/ansible/pull/62014 iam_role : support managing max session duration and deleting the instance profile it creates
10:05:09 <gundalow> tremble: are they all AWS?
10:05:16 <tremble> yup
10:05:20 <gundalow> cool
10:06:33 <gundalow> #info label/pr_day open:35 Closed: 77
10:06:44 <gundalow> ^ Just making a note of how we are starting the day
10:07:39 <gundalow> tremble: your reasons for doing this as a single PR sound sensible
10:08:00 <gundalow> I personally don't know much about AWS or it's tests
10:10:33 <gundalow> tremble: why is iam_role `unsupported`?
10:11:23 <shaps> morning
10:11:35 <gundalow> #chair shaps
10:11:35 <zodbot> Current chairs: gundalow shaps tremble
10:11:37 <tremble> From what I understand, a) the cleanup pieces in aws-terminator don't exist yet  b) automatically testing roles has some security implications.
10:11:38 <gundalow> Morning shaps
10:12:48 <Xaroth> o7
10:12:51 <tremble> In theory I could write a PR which gave me full admin access to the Account and hijack the account.
10:14:04 <gundalow> #chair Xaroth
10:14:04 <zodbot> Current chairs: Xaroth gundalow shaps tremble
10:14:07 <gundalow> Hi Xaroth :)
10:20:02 <gundalow> tremble: hum, so I've looked through the 6 PRs and I don't think I can add much
10:20:14 <tremble> Fair enough
10:20:20 <gundalow> though some seem to be with jillr and shertel so I hope they'll get sorted soon
10:20:24 <misc> I guess it would make sense to ask aws for some kind of mock aws zone or something, since ansible is likely not the only tool facing this kind of issue
10:20:49 <gundalow> After AnsibleFest I think people will have more time
10:21:07 <gundalow> misc: that's a decent point, though I'm not aware of any contacts at AWS
10:21:24 <misc> gundalow: we had one guy at flock
10:21:35 <tremble> misc: We have an AWS account for testing, the issue is mostly how to keep everything clean and secure when testing things like AWS access.
10:21:53 <misc> and I am sure greg know people at AWS, since there is a few RHer there
10:22:26 <misc> (like Max spevack)
10:22:57 <misc> tremble: yeah, a way to reset the account would be useful, and I kind aimagine they do have that internally, assuming they have tests
10:24:30 <tremble> misc: In theory we could use "AWS Organizations" to dynamically create and destroy entire accounts but I have no idea how AWS (or InfoSec) would react.
10:26:10 <misc> tremble: we would need to do that outside of the test, so the test do not have access to that
10:26:33 <tremble> yup
10:27:16 <tremble> it also significantly increases the time to run the tests though, it can take 10-20 minutes for the account to be created.
10:27:46 <misc> account could be precreated
10:28:10 <tremble> yeah, accounts are (without anything in them) free.
10:28:20 <misc> depend how much it cost, but if you have idle account ready, and create as soon as you take one, I think that would be ok
10:28:24 <gundalow> Will needs others to help with this stuff, so I wonder if we should table this for post-Fest
10:28:36 * tremble nods
10:29:04 <gundalow> #action gundalow to follow up AWS discussion with others post-fest
10:29:11 <gundalow> #topic label/small_patch
10:29:22 <gundalow> small_patch = smallish PR, hopefully easier to review
10:30:20 <gundalow> https://github.com/ansible/ansible/pull/62531 docs: really clarify environment
10:31:01 <gundalow> How's ^ wording
10:32:11 <misc> mhh, i think it still rely on understanding how ansible work
10:32:19 <misc> so since I know, it sound logical
10:32:35 <tremble> I'd probably word this as "...this can only affect modules and not Ansible itself..."
10:32:46 <gundalow> tremble: aye, start with positive
10:34:12 <gundalow> >  A dictionary that gets converted into environment vars to be provided for the task upon execution. This can ONLY be used with modules. This isn't supported for any other type of plugins nor Ansible itself nor its configuration, it just sets the variables for the code responsible for executing the task.
10:34:29 <robertdebock> Yes, better.
10:34:36 <gundalow> hi robertdebock :)
10:34:39 <gundalow> #chair robertdebock
10:34:39 <zodbot> Current chairs: Xaroth gundalow robertdebock shaps tremble
10:34:45 <robertdebock> hi!
10:34:50 <gundalow> Thanks, I've added that as a suggestion
10:35:19 <gundalow> https://github.com/ansible/ansible/pull/62491 ovirt_host update force doc
10:36:37 <misc> someone already suggested a change yesterday
10:37:28 <robertdebock> Looks like this will be resolved by itself, such a fresh issue.
10:38:56 <gundalow> yup
10:39:14 <gundalow> https://github.com/ansible/ansible/pull/62482 Removing hyperized as maintainer
10:39:16 <robertdebock> John, can you give me the URL with filters that shows the issues we're addressing?
10:39:16 <gundalow> I'll merge that
10:40:40 <misc> robertdebock: https://github.com/ansible/ansible/labels/pr_day
10:41:08 <gundalow> ^ is what we've looked at
10:41:16 <gundalow> todo list: https://github.com/ansible/ansible/pulls?utf8=%E2%9C%93&q=-label%3Apr_day+is%3Apr+is%3Aopen+-label%3Abackport++label%3Asmall_patch+
10:41:17 <robertdebock> thanks misc!
10:42:20 <gundalow> https://github.com/ansible/ansible/pull/62330 nsupdate: Don't mention the Microsoft DNS server
10:43:23 <misc> mhh, why was the mention in the first place ?
10:43:31 <misc> cause if it did work in the past, that's a bug
10:43:40 <robertdebock> Seems not relevant.
10:43:43 <misc> and if it never worked, well, why did the doc said otherwise :/
10:43:47 <robertdebock> be right back.
10:44:12 <gundalow> https://technet.microsoft.com/en-us/library/cc961412.aspx is about Windows 2000
10:44:49 <misc> wow
10:44:51 <misc> yeah
10:46:28 <gundalow> merged
10:48:27 <gundalow> https://github.com/ansible/ansible/pull/61607 lxd_container: enables to set keys not present in existing config
10:51:33 <misc> seems a good change
10:52:20 <misc> even if it will have side effect of returning changed if someone is setting a value explictely instead of using the default, even if the value is the same
10:52:27 <misc> (but that would be hard to avoid)
10:53:00 <gundalow> Good spot, yup
10:53:01 <misc> mhh
10:53:05 <gundalow> added rebuild_merge
10:53:31 <misc> and yeah, before, it would have show a traceback
10:54:32 <misc> it could have been easy to test, if that didn't implie to write a whole set of tests for the module
10:54:40 <gundalow> hum, this GitHub query doesn't seem to be moving that well (I'm looking at more PRs than I'm pasting here)
10:54:41 <misc> (so dealing with lxd setup, etc, etc)
10:54:56 <gundalow> misc: aye, that's a fair chunk of work
10:55:39 <misc> I wonder if that would be worthwhile to tag bugfix with "easy to test" and then direct people to that as a start point for writing tests
10:55:57 <gundalow> I'm tempted to swap to PRs we could close and looking at them
10:55:58 <misc> (it could be useful for a hackfest)
10:56:23 <gundalow> I normally put some more time into PRs days in advance getting sensible GitHub queries, though it's all been FEST FEST FEST
10:57:01 <gundalow> misc: That's a decent idea
10:57:40 <gundalow> #action gundalow to look at creating some sort of easy_to_test label as a todo list of things that people can create integration tests for
10:57:47 <gundalow> #topic Deprecated PRs
10:57:52 <gundalow> #info https://github.com/ansible/ansible/pulls?q=is%3Aopen+label%3Adeprecated+is%3Apr
10:58:09 <gundalow> https://github.com/ansible/ansible/pull/31664 Add module ldap_attrs; deprecate ldap_attr
10:58:47 <gundalow> ^ seems to have gotten stuck
10:59:39 <Xaroth> I know that one
10:59:41 * Xaroth goes look
11:00:46 <Xaroth> might be an idea to ask if he needs help with something
11:01:06 <Xaroth> can't quickly glance what he's stuck with, and don't have a ldap test env close by to test his code against
11:01:42 <gundalow> yup drybjed has outlined the reasons, though I think it's just lack of reviews that's getting this in
11:02:28 <Xaroth> yeah
11:02:55 * gundalow /joins #debops to ask there
11:03:59 * gundalow -> lunch
11:04:12 <gundalow> What other queries could we go though to find stuff to close/
11:05:02 <tremble> gundalow: Would it be worth attacking the seriously stale stuff?
11:20:34 <robertdebock> Back.
11:37:30 <gundalow> yes, lets look at closing some old stuff
11:38:12 <gundalow> #topic needs_repo
11:38:36 <gundalow> We looked at https://github.com/ansible/ansible/labels/needs_repo two weeks ago, I think unless there has been any movement we can close stuff
11:39:54 <gundalow> https://github.com/ansible/ansible/pull/57779 I'll ask if this is good to merge as-is, otherise wil be clsoed
11:40:46 <Xaroth> sounds good to me
11:41:05 <gundalow> https://github.com/ansible/ansible/pull/54912
11:41:05 <gundalow> Create django_models.py
11:41:24 <gundalow> no update in 15 days, branch has been deleted. Closing
11:41:53 <Xaroth> agreed
11:42:00 <gundalow> https://github.com/ansible/ansible/pull/54167 Return MAC address from Supermicro and HP systems
11:42:57 <Xaroth> same, I'd say, no response in over two weeks
11:44:17 <gundalow> https://github.com/ansible/ansible/pull/51371 flatpak: Open subprocess in text stream mode
11:44:42 <gundalow> given `I have a Fedora 29 environment with Python 2 running now, but I'm still not able to reproduce this bug.` going to close
11:45:06 <Xaroth> agreed
11:45:37 <Xaroth> https://github.com/ansible/ansible/pull/42055  Update aws_api_gateway.py
11:45:42 <Xaroth> no update since Sep 2018
11:45:47 <Xaroth> that's... a year ago almost
11:45:52 <Xaroth> I'd say close that.
11:47:00 <Xaroth> https://github.com/ansible/ansible/pull/41653  add master_ssl_verify_identity to mysql_replication
11:47:11 <gundalow> I'll close aws_api_gateway
11:47:35 <Xaroth> no input from PR opener in over a year.. actually, all he did was open a PR, he didn't act on the CI errors that were triggered
11:48:35 <Xaroth> so I'd say close that as well
11:49:59 <gundalow> yup
11:50:08 <gundalow> https://github.com/ansible/ansible/pull/40411 Add support for GitHub Enterprise for github_deploy_key
11:50:51 <Xaroth> Great PR idea, but that branch has been deleted over a year ago (Jul 2018)
11:51:07 <gundalow> aye, which is a shame. Though I don't have GitLab so can't test it
11:51:18 <Xaroth> it's for GH enterprise
11:51:19 <Xaroth> not GL :P
11:51:21 <gundalow> and I doubt shaps has any free time
11:51:24 <gundalow> oh, also words
11:51:42 <Xaroth> but yeah, testability is an issue :|
11:54:34 <gundalow> closed
11:54:55 <gundalow> I'm getting happier closing stuff
11:55:04 <gundalow> https://github.com/ansible/ansible/pull/40003 Update cron.py
11:55:19 <gundalow> oh, they responded
11:56:57 <gundalow> hum, though doesn't sound like they have any time
12:02:36 <gundalow> closed
12:03:39 * resmo waves
12:04:25 <gundalow> hey resmo :) You joining PR review day/
12:04:55 <gundalow> https://github.com/ansible/ansible/pull/39715 elb_target_group: only drain all elb targets if targets is an empty list and modify_targets is true
12:05:13 <gundalow> Going to live that open a bit longer as it does seem useful
12:05:42 <gundalow> https://github.com/ansible/ansible/pull/32930 Windows Facts: add WinSystemLocale
12:05:58 <gundalow> oh, they replied. I'll pass this over to Windows people
12:07:00 <tremble> gundalow: I think we can close https://github.com/ansible/ansible/pull/22521
12:08:09 <gundalow> yup, closed, thanks :)
12:09:49 <gundalow> https://github.com/ansible/ansible/pull/32158 iptables: chain creation and deletion
12:09:49 <gundalow> They replied, so will leave it open
12:10:00 <gundalow> https://github.com/ansible/ansible/pull/32011
12:10:00 <gundalow> Fix paramter confusion between directory and passwordstore
12:10:25 <tremble> Same with https://github.com/ansible/ansible/pull/19868
12:12:00 <gundalow> yup, there is a replacement PR
12:12:02 <gundalow> Thanks
12:15:16 <gundalow> https://github.com/ansible/ansible/pull/32011 Fix paramter confusion between directory and passwordstore
12:16:03 <gundalow> Closed
12:16:22 <gundalow> https://github.com/ansible/ansible/pull/28606 haproxy: connect via TCP socket in addition to UNIX socket
12:16:42 <gundalow> closed
12:17:21 <gundalow> https://github.com/ansible/ansible/pull/28469 WIP: New module: diff (string, file or command output vs string, file or command output)
12:17:21 <gundalow> * WIP, no updates, gona close
12:20:55 <tremble> https://github.com/ansible/ansible/pull/24582 appears to be a candidate for closure
12:22:24 <tremble> https://github.com/ansible/ansible/pull/25176 Is really stale, I'm sure shertel wouldn't be offended if you closed it and she still wanted it around.
12:23:13 <shertel> +1
12:23:22 <gundalow> a wild shertel appears!
12:23:26 <gundalow> Thanks
12:24:19 <tremble> https://github.com/ansible/ansible/pull/25214 looks to be abandoned
12:25:54 <tremble> Similarly I suspect https://github.com/ansible/ansible/pull/25290 may be nailed to the perch rather than 'resting'.
12:26:49 <tremble> https://github.com/ansible/ansible/pull/25919 - was asked to resolve the conflict a year ago, looks abandoned.
12:28:06 <gundalow> I've asked will about dead vs resting
12:29:41 <tremble> https://github.com/ansible/ansible/pull/26268 - rebase request a year ago, no updates
12:30:44 <gundalow> yup, rest closed
12:33:40 <gundalow> https://github.com/ansible/ansible/pull/24690 closed
12:36:01 <Xaroth> You're on fire, gundalow
12:36:40 <gundalow> Xaroth: slow start
12:36:49 <gundalow> though tremble has been a machine
12:37:05 <gundalow> tremble: willthames replied and said https://github.com/ansible/ansible/pull/25290 is dead, so I'll close
12:37:13 <tremble> gundalow: :)
12:37:35 <tremble> I think he's trying to step back from AWS stuff, so I'm not surprised
12:37:41 <gundalow> yup
12:37:50 <gundalow> Totally understandable
12:38:33 <tremble> https://github.com/ansible/ansible/pull/30746 looks dead
12:38:46 <gundalow> We've closed 21 PRs
12:39:30 <gundalow> tremble: yup, I'll close it. mikeldr hasn't been around in a few years
12:44:22 <Xaroth> which label are we processing now?
12:45:24 <tremble> https://github.com/ansible/ansible/pull/34022 looks to have been superceeded by #36641
12:45:58 <tremble> Xaroth: Personally I'm going through the ancient AWS PRs since I have half an idea what I'm looking at...
12:46:05 * Xaroth nods
12:46:27 <Xaroth> If this project ever ends I might be adding some more items to that aws queue :P
12:46:50 <tremble> Xaroth: I have been, which is why I'm attacking it ;)
12:47:10 <Xaroth> yeah, I wanted to, but this deadline's been so insane it's unbelievable
12:47:20 <Xaroth> working 250+ hours a month just for a shot of completion :|
12:48:22 <gundalow> urgh, that doesn't sound fun or healthy
12:49:34 <Xaroth> I'm hoping the bonus will compensate for part of both :P
12:49:38 <Xaroth> ... and a well earned vacation
12:49:50 <gundalow> For things to close I wonder if `is:open label:needs_rebase label:feature label:support:community sort:created-asc `
12:49:51 <gundalow> * Start with oldest Community Feature PRs that need rebase (most likely abandoned)
12:51:16 <Xaroth> https://github.com/ansible/ansible/pull/12090 4 years old, no repo, no response from owner in over a year;  close it I'd say
12:52:11 <Xaroth> https://github.com/ansible/ansible/pull/13612 same, over two years no response, close it I'd say
12:52:29 <Xaroth> maybe close it with a message that it can be re-opened if they wish
12:52:39 <Xaroth> but those are PRs for <2.5
12:53:09 <Xaroth> https://github.com/ansible/ansible/pull/14109 << Same as above
12:54:19 <Xaroth> It is probably easier to find cases that do not fall under this :P
12:56:19 <gundalow> ^ closed
12:56:38 <Xaroth> https://github.com/ansible/ansible/pull/14235, https://github.com/ansible/ansible/pull/17989, https://github.com/ansible/ansible/pull/18596, https://github.com/ansible/ansible/pull/19123, https://github.com/ansible/ansible/pull/19695 all the same as the others.
12:57:11 <gundalow> I've got to drop off for ~30 minutes or so, I'll catch up when I'm back
12:57:23 <gundalow> Though do feel free to step away as well :)
12:59:47 <Xaroth> https://github.com/ansible/ansible/pull/20189 same as above
13:00:14 <Xaroth> https://github.com/ansible/ansible/pull/20377  and   https://github.com/ansible/ansible/pull/20630 have had recent ( <1y ) activity.. might be an idea to ask for a new maintainer of this issue?
13:00:52 <tremble> https://github.com/ansible/ansible/pull/31427 looks to have been mooted by #43113
13:01:24 <Xaroth> ^ agreed
13:02:06 <Xaroth> https://github.com/ansible/ansible/pull/20920  was abandoned probably because it was waiting on review, suggest poking and de-assigning it.
13:03:22 <Xaroth> https://github.com/ansible/ansible/pull/20959 << no response in >1y, I'd say message about re-opening, and closing
13:05:35 <Xaroth> https://github.com/ansible/ansible/pull/21646 perhaps something to poke will about
13:13:52 <tremble> https://github.com/ansible/ansible/pull/38965 mooted by #47217 ( I think )
13:30:30 <tremble> https://github.com/ansible/ansible/pull/37716 changes requested a 15 months ago, no response.
13:32:59 <tremble> https://github.com/ansible/ansible/pull/37475 changes requested 1 year ago, no response.
13:42:21 <tremble> https://github.com/ansible/ansible/pull/36035 mooted by https://github.com/ansible/ansible/pull/47217
13:57:59 * gundalow returns
14:02:37 <gundalow> I'm going through the above and closing, thanks
14:09:00 <tremble> gundalow: Dumb question - how long should I leave it before starting to poke the Core AWS/Testing folks again?  I know they're snowed under right now with 2.9 related stuff
14:09:47 <gundalow> tremble: that's a valid question https://docs.ansible.com/ansible/devel/roadmap/ROADMAP_2_9.html says release 2019-10-10, so ~week  after that once people have caught up on sleep?
14:10:10 <tremble> ok, so a month or so.
14:10:30 <gundalow> realistically, yup
15:04:47 * tremble head out for the day, might pop back in later
15:05:00 <gundalow> tremble: You've been amazing help, thank you
15:05:20 <gundalow> I need to finish preparing for AnsibleFest, so will most likely end soon
15:06:05 <tremble> gundalow: my logic, if I can clear some of the crud, just maybe folks like shertel, jillr and mattclay will have more time to help with the things I care about ;)
15:06:21 <gundalow> Exactly, that's my plan
15:06:32 <gundalow> Plus there is the motivation of seeing the backlog drop
15:06:51 <tremble> At some point the issue queue needs attacking too :/
15:07:08 <gundalow> Yup, though I've been ignoring that
15:07:39 <gundalow> #info We've closed 36 PRs
15:07:51 <shertel> Thanks for investing time in all this, tremble.
15:08:12 <gundalow> well, most likely a few more as I've no doubt forgotten to add label/pr_day to a few
15:33:23 <gundalow> tributarian: Xaroth shertel misc felixfontein shaps Thankyou for your time today
15:33:25 <gundalow> #endmeeting