ec2 inventory: python 3.7 compatibility

[siwyd]

SUMMARY

ec2 inventory script was throwing errors when using Python 3.7:

❯ ./ec2.py --list  
Traceback (most recent call last):
  File "./ec2.py", line 1711, in <module>
    Ec2Inventory()
  File "./ec2.py", line 267, in __init__
    self.read_settings()
  File "./ec2.py", line 321, in read_settings
    config = configparser.ConfigParser(DEFAULTS)
  File "/usr/lib/python3.7/configparser.py", line 638, in __init__
    self._read_defaults(defaults)
  File "/usr/lib/python3.7/configparser.py", line 1216, in _read_defaults
    self.read_dict({self.default_section: defaults})
  File "/usr/lib/python3.7/configparser.py", line 753, in read_dict
    self.set(section, key, value)
  File "/usr/lib/python3.7/configparser.py", line 1197, in set
    self._validate_value_types(option=option, value=value)
  File "/usr/lib/python3.7/configparser.py", line 1182, in _validate_value_types
    raise TypeError("option values must be strings")
TypeError: option values must be strings

This changes the ConfigParser options that hava a None value to empty strings instead.

ISSUE TYPE

• Bugfix Pull Request

COMPONENT NAME

contrib ec2 dynamic inventory

ANSIBLE VERSION

ansible 2.6.2
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/home/hb/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 3.7.0 (default, Jul 15 2018, 10:44:58) [GCC 8.1.1 20180531]

ADDITIONAL INFORMATION

N/A

[ansibot]

cc @ryansb @s-hertel @willthames
click here for bot help

[Jackyjjc]

What about adding allow_no_value=True to the constructor of ConfigParser? Then we can keep the None values

[siwyd]

Sure, that can work too I guess. Although I see now that there was already a mix of empty strings and None, for instance:

'route53_excluded_zones': '',
'route53_hostnames': None,

So maybe best to pick one or the other and make it consistent?

[siwyd]

Any feedback on this?

[bqbn]

We ran into the same issue today.

Can we fix this please?

[madhead]

Until the fix is available, try replacing:

if six.PY3:
    config = configparser.ConfigParser(DEFAULTS)
else:
    config = configparser.SafeConfigParser(DEFAULTS)

with

if six.PY3:
    config = configparser.ConfigParser(DEFAULTS, allow_no_value=True)
else:
    config = configparser.SafeConfigParser(DEFAULTS, allow_no_value=True)

in ec2.py

[siwyd]

Happy to finish this PR one way or another, but need some feedback from somebody that has magic merge powers... @dagwieers I'm sure you're still lurking about somewhere? :)

[dagwieers]

Well, I wouldn't know if this is the right fix, or if @madhead 's suggestion is a better idea, honestly.

[willthames]

Merged, thanks for the fix (and apology for the delay reviewing/merging)

[willthames]

Raised a backport request to get this into upcoming 2.7 release (otherwise it would wait until 2.8)