19:06:39 <abadger1999> #startmeeting Ansible Core Team IRC Meeting
19:06:40 <zodbot> Meeting started Tue Mar 13 19:06:39 2018 UTC.  The chair is abadger1999. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:06:40 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
19:06:40 <zodbot> The meeting name has been set to 'ansible_core_team_irc_meeting'
19:06:42 <abadger1999> Any community people around that want to participate i nthe core irc meeting today?
19:07:17 <abadger1999> On the agenda we only have one PR that bcoca promised to review last week
19:07:28 <abadger1999> and a piece of information.
19:07:39 <abadger1999> #info Agenda: https://github.com/ansible/ansible/pull/36357
19:07:49 <abadger1999> #undo
19:07:49 <zodbot> Removing item from minutes: INFO by abadger1999 at 19:07:39 : Agenda: https://github.com/ansible/ansible/pull/36357
19:07:53 <abadger1999> #info Agenda: https://github.com/ansible/community/issues/296#issuecomment-370646587
19:08:34 <abadger1999> #info Ansible 2.5.0 Release Candidate 2 is now available! Get it on PyPI: pip install ansible==2.5.0rc2, https://releases.ansible.com/ansible/, or GitHub.
19:16:49 <svg> o/
19:16:55 * svg is lurking
19:17:26 <simondodsley> #topic review a couple of PRs .
19:17:47 <abadger1999> #chair svg simondodsley
19:17:47 <zodbot> Current chairs: abadger1999 simondodsley svg
19:17:54 <abadger1999> simondodsley: what PRs do you have?
19:18:00 <simondodsley> Put these up a few weeks ago. Would like to get them merged. https://github.com/ansible/ansible/pull/36761 https://github.com/ansible/ansible/pull/37043
19:18:14 <simondodsley> new facts for iSCSI and FC WWN
19:18:40 <simondodsley> iSCSI was discussed some weeks ago. Did the requested changes
19:18:45 <simondodsley> FC WWN is a new one
19:20:18 <abadger1999> alikins, bcoca: https://github.com/ansible/ansible/pull/36761  <=== I think the only comment we had on this one was that it should go into something besides minimal.  It's now in Network.  Does it look good to merge now?
19:21:55 <sdoran> `aixcmd` is using a hardcoded path to the binary.
19:22:23 <simondodsley> which is the way all the aix calls are made elsewhere in Ansible
19:22:33 <sdoran> Would be more resilient to use `get_bin_path` if possible.
19:23:14 <simondodsley> was going to do that but went with the same methodology as other AIX calls
19:23:28 <simondodsley> its a core AIX binary and will never change location
19:23:36 <sdoran> Ok. Makes sense. Just something to keep in mind if it becomes an issue in the future.
19:24:16 <simondodsley> there will be lots to change if that breaks :)
19:30:03 <sdoran> Aside from `get_bin_path`, I'd say this looks good to merge. All other items were addressed.
19:30:58 <sdoran> Example output is very handy to see. Thanks for that, simondodsley .
19:32:50 <abadger1999> simondodsley: Is there a reason you start with this: iscsi_facts['iscsi_iqn'] = {}   then overwrite it with a list? iscsi_facts['iscsi_iqn'] = line.split('=', 1)[1]
19:34:11 <simondodsley> there probably was, but I don't remember what it was right now
19:34:45 <abadger1999> Maybe you started with Linux and thought it should have k=v pairs but then the other platforms didn't have keys
19:35:50 <simondodsley> i admit i started with my favorite OS - linux and then went downhill from then on
19:36:38 <abadger1999> Looking at the Linux one some more, the code is also looping through lines in /etc/iscsi/initiatorname.iscsi but the last line with InitiatorName= is overwriting anything previous.  Perhaps it should either append to a list or it should break after the line is found?
19:38:19 <simondodsley> the format of the file is hard defined by the iSCSI standard, but different OS's add different comments (or not) in the file, so we have to check for InitiatorName= (note the equals sign) so we can't just check for InitiatorName
19:38:40 <simondodsley> there can onl be one IQN per host
19:39:42 <abadger1999> Cool.  So if you add a break after we've processed that line, I think that will be good then.
19:39:58 <simondodsley> that line will always be last - its part of the standard
19:40:16 <abadger1999> Mostly so that people understand what hte code is doing.
19:40:35 <abadger1999> Hmm.. if it's always last, a minor optimization would be to search the lines in reverse order.
19:41:31 <simondodsley> ok - i'll add in a break - you never know if another OS will break the standard
19:41:41 <simondodsley> not unheard of...
19:42:58 <abadger1999> Cool.
19:43:08 <abadger1999> lines = get_file_content('/etc/iscsi/initiatorname.iscsi', '').splitlines()
19:43:21 <abadger1999> lines.reverse()
19:43:24 <abadger1999> for line in lines:
19:43:56 <abadger1999> or one-line:
19:44:10 <abadger1999> for line in reversed(get_file_content('/etc/iscsi/initiatorname.iscsi', '').splitlines()):
19:45:34 <abadger1999> Okay cool, make that change and probably... iscsi_facts['iscsi_iqn'] = None or iscsi_facts['iscsi_iqn'] = "" and I'll merge that one.
19:46:11 <simondodsley> on it. I'll ping when it's done
19:46:17 <simondodsley> Thoughts on the second PR?
19:46:27 <simondodsley> https://github.com/ansible/ansible/pull/37043
19:48:03 <abadger1999> I have never used fibrechannel so I may be asking silly questions... are there more facts that should eventually be grouped together with that?  does wnn exactly map to wnn?
19:48:10 <alikins> does that windows/powershell bit work?
19:48:59 <jborean93> It wouldn't run
19:49:04 <jborean93> We don't use setup.py
19:49:18 <simondodsley> from an FC perspective the only thing that people will need are the port names. There is a node name, but not used for anything. the port names are the ones required for zoning to external storage
19:49:21 <jborean93> So not necessary to have in that PR
19:50:19 <abadger1999> Okay, so that's the third thing to do for 36761 (remove the powershell/windows bit)
19:50:42 <simondodsley> ok - it will be gone
19:51:01 <abadger1999> Alright, alikins is https://github.com/ansible/ansible/pull/37043/files okay with you?  It looks pretty straightforward to me.
19:53:11 <alikins> not really. there is a mechanism for doing per platform facts it could be using. Also not sure it needs to be in the base fact set. Also needs tests and sample data.
19:54:47 <jtanner> agree on the last one
19:55:03 <simondodsley> so what is that mechanism? Why is it not a base fact? It's as base as any network card in a server
19:55:18 <jtanner> simondodsley: what distros has that been tested on?
19:55:35 <simondodsley> centos and ubuntu
19:58:29 <alikins> simondodsley: create a platform specific collector like https://github.com/ansible/ansible/blob/devel/lib/ansible/module_utils/facts/network/aix.py#L142
20:00:41 <simondodsley> I won't be able to test it then as I don't have an AIX host anymore
20:03:17 <abadger1999> Alright, we've hit our time limit.
20:03:33 <abadger1999> alikins, simondodsley are you two able to keep discussing this in #ansible-devel?
20:04:11 <abadger1999> #action abadger1999 to merge https://github.com/ansible/ansible/pull/36761 once simondodsley addresses comments from this meeting
20:04:37 <abadger1999> #action alikins and simondodsley to discuss how to improve https://github.com/ansible/ansible/pull/37043 (FibreChannel Facts PR)
20:05:43 <bcoca> dmit ... why do i keep comming at wrong hour?
20:06:15 <jborean93> DST?
20:06:19 <abadger1999> bcoca: we even changed the legal time to try to make it work for you ;-)
20:06:41 <abadger1999> Okay, I'll close the meeting in 60 seconds if there's nothing else.
20:45:59 <abadger1999> #endmeeting