15:59:57 <Qalthos> #startmeeting Ansible Network Working Group
15:59:57 <zodbot> Meeting started Wed May 29 15:59:57 2019 UTC.
15:59:57 <zodbot> This meeting is logged and archived in a public location.
15:59:57 <zodbot> The chair is Qalthos. Information about MeetBot at http://wiki.debian.org/MeetBot.
15:59:57 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
15:59:57 <zodbot> The meeting name has been set to 'ansible_network_working_group'
16:00:47 <Qalthos> #chair ganeshrn ikhan justjais pabelanger privateip trishnag dmellado
16:00:47 <zodbot> Current chairs: Qalthos dmellado ganeshrn ikhan justjais pabelanger privateip trishnag
16:01:19 <Qalthos> There's a lot of issues on the agenda today, so I'll try to get through as many as I can
16:02:09 <Qalthos> First, though a slight review of the PR review process
16:02:24 <Qalthos> Specifically https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md#when-will-your-pull-request-be-merged
16:03:46 <Qalthos> If you are a maintainer of a platform, as most of the people adding PRs to this meeting's agenda are, please make sure that the maintainer list in https://github.com/ansible/ansible/blob/devel/.github/BOTMETA.yml is up to date
16:05:17 <Qalthos> and especially if you  have more than one maintainer, help out the people reviewing your PR by using shipit
16:07:07 <Qalthos> The more feedback you can get on a PR before someone from core has to look at it, the quicker that process will be
16:07:19 <Qalthos> Now, then
16:07:51 <Qalthos> #link https://github.com/ansible/community/labels/network is where the agenda for this meeting is always available
16:08:00 <Qalthos> And it's a long one, so let's get moving
16:08:17 <Qalthos> #topic Meraki
16:08:36 <mrproper> I listed four PRs which need review and merge if reviewer is happy.
16:08:43 <mrproper> https://github.com/ansible/ansible/pull/56818
16:09:25 <Qalthos> Anything in particular to mention about any of them? Do we ahve all the prerequisite PRs in now?
16:09:57 <mrproper> Yes. All the prereqs should be in place. I don't think there's anything interesting about any of them. 56818 and 48971 are on the same codebase, but I tested a merge earlier and there weren't any conflicts.
16:10:23 <mrproper> 56929 is a new module and it hasn't yet been reviewed, so I don't necessarily expect it's perfect.
16:11:17 <Qalthos> 56818 is commenting out tests?
16:12:19 <mrproper> I didn't see that. Let me review that.
16:13:11 <mrproper> I can test tonight and update PR.
16:16:37 <Qalthos> Okay, will keep on to review pile and move on then
16:17:05 <mrproper> https://github.com/ansible/ansible/pull/48971
16:17:09 <Qalthos> #topic CloudEngine PRs
16:17:50 <xuxiaowei0512> https://github.com/ansible/ansible/pull/57123
16:17:57 <xuxiaowei0512> add a new module
16:18:03 <xuxiaowei0512> want to use ftp
16:23:13 <Qalthos> xuxiaowei0512: I'm not sure I follow what this module has to do with CE specifically
16:23:49 <Qalthos> It looks like it just transfers a file to or from a remote FTP server? Is there more to that?
16:24:25 <xuxiaowei0512> yes, A new module uses FTP to transfer files
16:24:59 <xuxiaowei0512> Some scenarios must be used
16:26:19 <Qalthos> xuxiaowei0512: can you not do this with https://docs.ansible.com/ansible/latest/modules/get_url_module.html
16:27:02 <xuxiaowei0512> Some network environments can only use FPT
16:27:23 <Qalthos> We already have some support for FTP transfers, and if you need more, it almost certainly should not be in a networking platform module
16:27:38 <xuxiaowei0512> CloudEngine cant not use that
16:27:41 <Qalthos> xuxiaowei0512: get_url supports ftp:// urls
16:28:17 <Qalthos> (somewhat opaquely, but still)
16:28:46 <xuxiaowei0512> CE software does not support it
16:29:48 <xuxiaowei0512> I tested this method many times in our lab
16:31:11 <xuxiaowei0512> I've noticed that way before, but it's useless. So we developed a FTP module
16:31:43 <Qalthos> xuxiaowei0512: regardless, if you are looking for expanded FTP capabilities, either a PR to get_url or an new module in net_tools would be more appropriate
16:33:33 <Qalthos> The next two PRs look reasonable
16:34:02 <Qalthos> I'm going to skip to 57057 for a bit
16:34:38 <Qalthos> xuxiaowei0512: Are terminal_initial_prompt and terminal_initial_answer actually used anywhere?
16:35:30 <Qalthos> Oh, nevermind, I see how this works now
16:35:55 <Qalthos> 57057 is fine, too
16:36:04 <xuxiaowei0512> thank you
16:37:04 <Qalthos> As for the snmp PR, that is gonna need a more thorough review, and we need to move on. I'll see that someone looks over it
16:37:43 <xuxiaowei0512> Ok
16:38:25 <Qalthos> #topic onyx_telemetry_agent https://github.com/ansible/ansible/pull/57066
16:38:58 <Qalthos> If you're in here, I don't know what your IRC handle is
16:40:49 <Qalthos> Okay then, movin on
16:41:28 <Qalthos> #topic Avi PRs
16:43:54 <chaitanyaavi> We have added few new modules and split avi_credentials due to no log handling
16:48:51 <Qalthos> chaitanyaavi: so let's get into that avi_credentials thing for a bit
16:50:45 <Qalthos> What exactly should be in avi_login_info?
16:51:43 <chaitanyaavi> username, tenant, controller ip, avi_version etc can go in avi_login_info
16:52:01 <Qalthos> Is it a consistent enough set of fields that you could use suboptions here?
16:52:05 <Qalthos> (https://docs.ansible.com/ansible/latest/dev_guide/developing_modules_documenting.html)
16:53:56 <Qalthos> For that matter, you could instead keep avi_credentials unified, document the various suboptions, and tag only the sensitive ones as no_log
16:55:36 <chaitanyaavi> Ok I will check and update the PR
17:08:10 <Qalthos> As for the module PRs, there are a few notes, and none of them appear to have tests
17:11:54 <chaitanyaavi> we will add the tests are there any guidelines for the tests ?
17:12:40 <Qalthos> chaitanyaavi: We do require unit tests (https://docs.ansible.com/ansible/latest/dev_guide/testing_units.html#testing-units) on all new modules.
17:14:11 <Qalthos> We are way over, and I don't see the last person in here anyway
17:14:23 <chaitanyaavi> Ok we will add those
17:14:40 <Qalthos> Thanks everyone for your time and patience
17:14:52 <Qalthos> #endmeeting