openstack/ansible-collections-openstack
Code Issues Proposed changes

Fix W504 and remove exclusion

Browse Source 
This is a topic where there are two points of view. While neither
is fundamentally better than the other in reality, what's best is
to not have any arguments about it. The tox.ini comments about 503
and 504 that were in place make the argument that:

  - 503 is intended to be disabled and 504 enabled by default
  - Donald Knuth believes 504 is the right way

Since Donald Knuth is smarter than all of us, align with 504, match
the comments in the file and turn on enforcement to keep it that way.

Change-Id: I92d4d1e82935e30ae42a0e14e641cbe36fd6e811
This commit is contained in:
Monty Taylor 2020-02-27 09:21:21 -06:00
parent 72ba76f156
commit 92d7d7caeb
13 changed files with 96 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
plugins/inventory/openstack.py
View File

@ -184,8 +184,10 @@ class InventoryModule(BaseInventoryPlugin, Constructable, Cacheable):
if not source_data:
clouds_yaml_path = self._config_data.get('clouds_yaml_path')
if clouds_yaml_path:
config_files = (clouds_yaml_path +
client_config.CONFIG_FILES)
config_files = (
clouds_yaml_path
+ client_config.CONFIG_FILES
)
else:
config_files = None

10
plugins/modules/os_floating_ip.py
View File

@ -185,10 +185,12 @@ def main():
network_id = None
# check if we have floating ip on given nat_destination network
if nat_destination:
nat_floating_addrs = [addr for addr in server.addresses.get(
cloud.get_network(nat_destination)['name'], [])
if addr['addr'] == public_ip and
addr['OS-EXT-IPS:type'] == 'floating']
nat_floating_addrs = [
addr for addr in server.addresses.get(
cloud.get_network(nat_destination)['name'], [])
if addr['addr'] == public_ip
and addr['OS-EXT-IPS:type'] == 'floating'
]
if len(nat_floating_addrs) == 0:
module.fail_json(msg="server {server} already has a "

12
plugins/modules/os_ironic.py
View File

@ -240,13 +240,17 @@ def main():
if not HAS_JSONPATCH:
module.fail_json(msg='jsonpatch is required for this module')
if (module.params['auth_type'] in [None, 'None'] and
module.params['ironic_url'] is None):
if (
module.params['auth_type'] in [None, 'None']
and module.params['ironic_url'] is None
):
module.fail_json(msg="Authentication appears to be disabled, "
"Please define an ironic_url parameter")
if (module.params['ironic_url'] and
module.params['auth_type'] in [None, 'None']):
if (
module.params['ironic_url']
and module.params['auth_type'] in [None, 'None']
):
module.params['auth'] = dict(
endpoint=module.params['ironic_url']
)

12
plugins/modules/os_ironic_inspect.py
View File

@ -106,13 +106,17 @@ def main():
module_kwargs = openstack_module_kwargs()
module = AnsibleModule(argument_spec, **module_kwargs)
if (module.params['auth_type'] in [None, 'None'] and
module.params['ironic_url'] is None):
if (
module.params['auth_type'] in [None, 'None']
and module.params['ironic_url'] is None
):
module.fail_json(msg="Authentication appears to be disabled, "
"Please define an ironic_url parameter")
if (module.params['ironic_url'] and
module.params['auth_type'] in [None, 'None']):
if (
module.params['ironic_url']
and module.params['auth_type'] in [None, 'None']
):
module.params['auth'] = dict(
endpoint=module.params['ironic_url']
)

36
plugins/modules/os_ironic_node.py
View File

@ -194,11 +194,15 @@ def _check_set_power_state(module, cloud, node):
cloud.set_machine_power_off(node['uuid'])
module.exit_json(changed=True, msg="Power requested off")
if 'power off' in str(node['power_state']):
if (_is_false(module.params['power']) and
_is_false(module.params['state'])):
if (
_is_false(module.params['power'])
and _is_false(module.params['state'])
):
return False
if (_is_false(module.params['power']) and
_is_false(module.params['state'])):
if (
_is_false(module.params['power'])
and _is_false(module.params['state'])
):
module.exit_json(
changed=False,
msg="Power for node is %s, node must be reactivated "
@ -207,8 +211,10 @@ def _check_set_power_state(module, cloud, node):
# In the event the power has been toggled on and
# deployment has been requested, we need to skip this
# step.
if (_is_true(module.params['power']) and
_is_false(module.params['deploy'])):
if (
_is_true(module.params['power'])
and _is_false(module.params['deploy'])
):
# Node is powered down when it is not awaiting to be provisioned
cloud.set_machine_power_on(node['uuid'])
return True
@ -234,19 +240,25 @@ def main():
module_kwargs = openstack_module_kwargs()
module = AnsibleModule(argument_spec, **module_kwargs)
if (module.params['auth_type'] in [None, 'None'] and
module.params['ironic_url'] is None):
if (
module.params['auth_type'] in [None, 'None']
and module.params['ironic_url'] is None
):
module.fail_json(msg="Authentication appears disabled, Please "
"define an ironic_url parameter")
if (module.params['ironic_url'] and
module.params['auth_type'] in [None, 'None']):
if (
module.params['ironic_url']
and module.params['auth_type'] in [None, 'None']
):
module.params['auth'] = dict(
endpoint=module.params['ironic_url']
)
if (module.params['config_drive'] and
not isinstance(module.params['config_drive'], (str, dict))):
if (
module.params['config_drive']
and not isinstance(module.params['config_drive'], (str, dict))
):
config_drive_type = type(module.params['config_drive'])
msg = ('argument config_drive is of type %s and we expected'
' str or dict') % config_drive_type

10
plugins/modules/os_nova_host_aggregate.py
View File

@ -76,10 +76,12 @@ def _needs_update(module, aggregate):
if module.params['availability_zone'] is not None:
new_metadata['availability_zone'] = module.params['availability_zone']
if ((module.params['name'] != aggregate.name) or
(module.params['hosts'] is not None and set(module.params['hosts']) != set(aggregate.hosts)) or
(module.params['availability_zone'] is not None and module.params['availability_zone'] != aggregate.availability_zone) or
(module.params['metadata'] is not None and new_metadata != aggregate.metadata)):
if (
(module.params['name'] != aggregate.name)
or (module.params['hosts'] is not None and set(module.params['hosts']) != set(aggregate.hosts))
or (module.params['availability_zone'] is not None and module.params['availability_zone'] != aggregate.availability_zone)
or (module.params['metadata'] is not None and new_metadata != aggregate.metadata)
):
return True
return False

6
plugins/modules/os_port.py
View File

@ -252,8 +252,10 @@ def _needs_update(module, port, cloud):
if module.params[key] is not None and module.params[key] != port[key]:
return True
for key in compare_list:
if module.params[key] is not None and (set(module.params[key]) !=
set(port[key])):
if (
module.params[key] is not None
and set(module.params[key]) != set(port[key])
):
return True
for key in compare_list_dict:

33
plugins/modules/os_security_group_rule.py
View File

@ -223,9 +223,13 @@ def _ports_match(protocol, module_min, module_max, rule_min, rule_max):
module_min = None
module_max = None
if ((module_min is None and module_max is None) and
(rule_min and int(rule_min) == 1 and
rule_max and int(rule_max) == 65535)):
if (
(module_min is None and module_max is None)
and (
rule_min and int(rule_min) == 1
and rule_max and int(rule_max) == 65535
)
):
# (None, None) == (1, 65535)
return True
@ -254,16 +258,19 @@ def _find_matching_rule(module, secgroup, remotegroup):
remote_group_id = remotegroup['id']
for rule in secgroup['security_group_rules']:
if (protocol == rule['protocol'] and
remote_ip_prefix == rule['remote_ip_prefix'] and
ethertype == rule['ethertype'] and
direction == rule['direction'] and
remote_group_id == rule['remote_group_id'] and
_ports_match(protocol,
module.params['port_range_min'],
module.params['port_range_max'],
rule['port_range_min'],
rule['port_range_max'])):
if (
protocol == rule['protocol']
and remote_ip_prefix == rule['remote_ip_prefix']
and ethertype == rule['ethertype']
and direction == rule['direction']
and remote_group_id == rule['remote_group_id']
and _ports_match(
protocol,
module.params['port_range_min'],
module.params['port_range_max'],
rule['port_range_min'],
rule['port_range_max'])
):
return rule
return None

6
plugins/modules/os_server.py
View File

@ -572,8 +572,10 @@ def _check_security_groups(module, cloud, server):
# server security groups were added to shade in 1.19. Until then this
# module simply ignored trying to update security groups and only set them
# on newly created hosts.
if not (hasattr(cloud, 'add_server_security_groups') and
hasattr(cloud, 'remove_server_security_groups')):
if not (
hasattr(cloud, 'add_server_security_groups')
and hasattr(cloud, 'remove_server_security_groups')
):
return changed, server
module_security_groups = set(module.params['security_groups'])

7
plugins/modules/os_subnet.py
View File

@ -272,8 +272,11 @@ def main():
if state == 'present':
if not module.params['network_name']:
module.fail_json(msg='network_name required with present state')
if (not module.params['cidr'] and not use_default_subnetpool and
not extra_specs.get('subnetpool_id', False)):
if (
not module.params['cidr']
and not use_default_subnetpool
and not extra_specs.get('subnetpool_id', False)
):
module.fail_json(msg='cidr or use_default_subnetpool or '
'subnetpool_id required with present state')

6
plugins/modules/os_user.py
View File

@ -150,8 +150,10 @@ def _needs_update(params_dict, user):
# We don't get password back in the user object, so assume any supplied
# password is a change.
if (params_dict['password'] is not None and
params_dict['update_password'] == 'always'):
if (
params_dict['password'] is not None
and params_dict['update_password'] == 'always'
):
return True
return False

4
tests/unit/mock/yaml_helper.py
View File

@ -106,8 +106,8 @@ class YamlTestUtils(object):
assert yaml_string == yaml_string_obj_from_stream
assert yaml_string == yaml_string_obj_from_stream == yaml_string_obj_from_string
assert (yaml_string == yaml_string_obj_from_stream == yaml_string_obj_from_string == yaml_string_stream_obj_from_stream ==
yaml_string_stream_obj_from_string)
assert (yaml_string == yaml_string_obj_from_stream == yaml_string_obj_from_string == yaml_string_stream_obj_from_stream
== yaml_string_stream_obj_from_string)
assert obj == obj_from_stream
assert obj == obj_from_string
assert obj == yaml_string_obj_from_stream

2
tox.ini
View File

@ -51,7 +51,7 @@ commands = {posargs}
# H4 are rules for docstrings. Maybe we should clean them?
# E501,E402,H301,H236,F401 are ignored so we can import the existing
# modules unchanged and then clean them in subsequent patches.
ignore = W503,H4,E501,E402,H301,H236,F401,W504
ignore = W503,H4,E501,E402,H301,H236,F401
show-source = True
exclude=.venv,.git,.tox,dist,doc,*lib/python*,*egg,build,ansible_collections