Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various pylint fixups #2221

Merged
3 changes: 3 additions & 0 deletions changelogs/fragments/20250127-pylint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
---
minor_changes:
- various modules - linting fixups (https://github.com/ansible-collections/community.aws/pull/2221).
2 changes: 1 addition & 1 deletion plugins/connection/aws_ssm.py
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@
try:
import boto3
from botocore.client import Config
except ImportError as e:
except ImportError:
pass

from functools import wraps
Expand Down
2 changes: 1 addition & 1 deletion plugins/module_utils/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ def _flush_update(self):

if not self.module.check_mode:
self._do_update_resource()
response = self._wait_for_update()
self._wait_for_update()
self.updated_resource = self.get_resource()
else: # (CHECK_MODE)
self.updated_resource = self._normalize_resource(self._generate_updated_resource())
Expand Down
20 changes: 10 additions & 10 deletions plugins/module_utils/ec2.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,15 @@ class BaseEc2Manager(Ec2Boto3Mixin, BaseResourceManager):
# If the resource supports using "TagSpecifications" on creation we can
TAGS_ON_CREATE = "TagSpecifications"

def __init__(self, module, id=None):
def __init__(self, module, resource_id=None):
r"""
Parameters:
module (AnsibleAWSModule): An Ansible module.
"""
super(BaseEc2Manager, self).__init__(module)
self.client = self._create_client()
self._tagging_updates = dict()
self.resource_id = id
self.resource_id = resource_id

# Name parameter is unique (by region) and can not be modified.
if self.resource_id:
Expand Down Expand Up @@ -97,22 +97,22 @@ def _paginated_describe_tags(self, **params):
return paginator.paginate(**params).build_full_result()

@Boto3Mixin.aws_error_handler("list tags on resource")
def _describe_tags(self, id=None):
if not id:
id = self.resource_id
filters = ansible_dict_to_boto3_filter_list({"resource-id": id})
def _describe_tags(self, resource_id=None):
if not resource_id:
resource_id = self.resource_id
filters = ansible_dict_to_boto3_filter_list({"resource-id": resource_id})
tags = self._paginated_describe_tags(Filters=filters)
return tags

def _get_tags(self, id=None):
if id is None:
id = self.resource_id
def _get_tags(self, resource_id=None):
if resource_id is None:
resource_id = self.resource_id
# If the Tags are available from the resource, then use them
if self.TAGS_ON_RESOURCE:
tags = self._preupdate_resource.get("Tags", [])
# Otherwise we'll have to look them up
else:
tags = self._describe_tags(id=id)
tags = self._describe_tags(resource_id=resource_id)
return boto3_tag_list_to_ansible_dict(tags)

def _do_tagging(self):
Expand Down
4 changes: 2 additions & 2 deletions plugins/module_utils/networkfirewall.py
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,7 @@ def _do_update_resource(self):
params["RuleGroup"] = resource

if not self.module.check_mode:
response = self._update_rule_group(**params)
self._update_rule_group(**params)

return True

Expand Down Expand Up @@ -1349,7 +1349,7 @@ def _do_update_resource(self):
params["FirewallPolicy"] = resource

if not self.module.check_mode:
response = self._update_policy(**params)
self._update_policy(**params)

return True

Expand Down
5 changes: 1 addition & 4 deletions plugins/modules/application_autoscaling_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -297,10 +297,7 @@
# Merge the results of the scalable target creation and policy deletion/creation
# There's no risk in overriding values since mutual keys have the same values in our case
def merge_results(scalable_target_result, policy_result):
if scalable_target_result["changed"] or policy_result["changed"]:
changed = True
else:
changed = False
changed = bool(scalable_target_result["changed"] or policy_result["changed"])

merged_response = scalable_target_result["response"].copy()
merged_response.update(policy_result["response"])
Expand Down
4 changes: 2 additions & 2 deletions plugins/modules/autoscaling_launch_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ def create_launch_config(connection, module):
)
except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
module.fail_json_aws(e, msg="Failed to get Security Group IDs")
except ValueError as e:
except ValueError:
module.fail_json(msg="Failed to get Security Group IDs", exception=traceback.format_exc())
user_data = module.params.get("user_data")
user_data_path = module.params.get("user_data_path")
Expand Down Expand Up @@ -571,7 +571,7 @@ def create_launch_config(connection, module):
try:
with open(user_data_path, "r") as user_data_file:
user_data = user_data_file.read()
except IOError as e:
except IOError:
module.fail_json(msg="Failed to open file for reading", exception=traceback.format_exc())

if volumes:
Expand Down
2 changes: 0 additions & 2 deletions plugins/modules/autoscaling_lifecycle_hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,8 +296,6 @@ def main():

connection = module.client("autoscaling")

changed = False

if state == "present":
create_lifecycle_hook(connection, module)
elif state == "absent":
Expand Down
6 changes: 3 additions & 3 deletions plugins/modules/autoscaling_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -494,10 +494,10 @@ def create_scaling_policy(connection, module):
changed = True
else:
policy = policies[0]
for key in params:
if params[key] != policy.get(key):
for key, old_value in params.items():
if old_value != policy.get(key):
changed = True
before[key] = params[key]
before[key] = old_value
after[key] = policy.get(key)

if changed:
Expand Down
3 changes: 0 additions & 3 deletions plugins/modules/batch_job_definition.py
Original file line number Diff line number Diff line change
Expand Up @@ -385,12 +385,9 @@ def manage_state(module, batch_client):
changed = False
current_state = "absent"
state = module.params["state"]
job_definition_name = module.params["job_definition_name"]
action_taken = "none"
response = None

check_mode = module.check_mode

# check if the job definition exists
current_job_definition = get_current_job_definition(module, batch_client)
if current_job_definition:
Expand Down
6 changes: 3 additions & 3 deletions plugins/modules/cloudfront_distribution.py
Original file line number Diff line number Diff line change
Expand Up @@ -1769,12 +1769,12 @@ def validate_origins(
origins,
default_origin_domain_name,
default_origin_path,
create_distribution,
create,
purge_origins=False,
):
try:
if origins is None:
if default_origin_domain_name is None and not create_distribution:
if default_origin_domain_name is None and not create:
if purge_origins:
return None
else:
Expand All @@ -1784,7 +1784,7 @@ def validate_origins(
else:
origins = []
self.validate_is_list(origins, "origins")
if not origins and default_origin_domain_name is None and create_distribution:
if not origins and default_origin_domain_name is None and create:
self.module.fail_json(
msg="Both origins[] and default_origin_domain_name have not been specified. Please specify at least one."
)
Expand Down
4 changes: 1 addition & 3 deletions plugins/modules/cloudfront_response_headers_policy.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,12 +166,10 @@ def find_response_headers_policy(self, name):

for policy in policies:
if policy["ResponseHeadersPolicy"]["ResponseHeadersPolicyConfig"]["Name"] == name:
policy_id = policy["ResponseHeadersPolicy"]["Id"]
# as the list_ request does not contain the Etag (which we need), we need to do another get_ request here
matching_policy = self.client.get_response_headers_policy(Id=policy["ResponseHeadersPolicy"]["Id"])
break
else:
matching_policy = None
matching_policy = None

return matching_policy
except (ClientError, BotoCoreError) as e:
Expand Down
4 changes: 0 additions & 4 deletions plugins/modules/codebuild_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -378,14 +378,12 @@ def do_update_project(client, params, formatted_params, found_project):


def create_or_update_project(client, params):
resp = {}
name = params["name"]
# clean up params
formatted_params = snake_dict_to_camel_dict(dict((k, v) for k, v in params.items() if v is not None))

# Check if project with that name already exists and if so update existing:
found = describe_project(client=client, name=name)
changed = False

if "name" not in found:
return do_create_project(client, params, formatted_params)
Expand All @@ -394,8 +392,6 @@ def create_or_update_project(client, params):


def update_project(client, params):
name = params["name"]

try:
resp = client.update_project(**params)
return resp
Expand Down
6 changes: 3 additions & 3 deletions plugins/modules/config_aggregation_authorization.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def resource_exists(client, module, params):

def create_resource(client, module, params, result):
try:
response = client.put_aggregation_authorization(
client.put_aggregation_authorization(
AuthorizedAccountId=params["AuthorizedAccountId"],
AuthorizedAwsRegion=params["AuthorizedAwsRegion"],
)
Expand All @@ -97,7 +97,7 @@ def update_resource(client, module, params, result):

if params != current_params:
try:
response = client.put_aggregation_authorization(
client.put_aggregation_authorization(
AuthorizedAccountId=params["AuthorizedAccountId"],
AuthorizedAwsRegion=params["AuthorizedAwsRegion"],
)
Expand All @@ -109,7 +109,7 @@ def update_resource(client, module, params, result):

def delete_resource(client, module, params, result):
try:
response = client.delete_aggregation_authorization(
client.delete_aggregation_authorization(
AuthorizedAccountId=params["AuthorizedAccountId"],
AuthorizedAwsRegion=params["AuthorizedAwsRegion"],
)
Expand Down
2 changes: 1 addition & 1 deletion plugins/modules/config_delivery_channel.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ def update_resource(client, module, params, result):

def delete_resource(client, module, params, result):
try:
response = client.delete_delivery_channel(DeliveryChannelName=params["name"])
client.delete_delivery_channel(DeliveryChannelName=params["name"])
result["changed"] = True
return result
except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
Expand Down
6 changes: 3 additions & 3 deletions plugins/modules/config_recorder.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def resource_exists(client, module, params):

def create_resource(client, module, params, result):
try:
response = client.put_configuration_recorder(ConfigurationRecorder=params)
client.put_configuration_recorder(ConfigurationRecorder=params)
result["changed"] = True
result["recorder"] = camel_dict_to_snake_dict(resource_exists(client, module, params))
return result
Expand All @@ -119,7 +119,7 @@ def update_resource(client, module, params, result):

if params != current_params["ConfigurationRecorders"][0]:
try:
response = client.put_configuration_recorder(ConfigurationRecorder=params)
client.put_configuration_recorder(ConfigurationRecorder=params)
result["changed"] = True
result["recorder"] = camel_dict_to_snake_dict(resource_exists(client, module, params))
return result
Expand All @@ -129,7 +129,7 @@ def update_resource(client, module, params, result):

def delete_resource(client, module, params, result):
try:
response = client.delete_configuration_recorder(ConfigurationRecorderName=params["name"])
client.delete_configuration_recorder(ConfigurationRecorderName=params["name"])
result["changed"] = True
return result
except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e:
Expand Down
3 changes: 1 addition & 2 deletions plugins/modules/config_rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ def update_resource(client, module, params, result):

def delete_resource(client, module, params, result):
try:
response = client.delete_config_rule(
client.delete_config_rule(
ConfigRuleName=params["ConfigRuleName"],
aws_retry=True,
)
Expand Down Expand Up @@ -202,7 +202,6 @@ def main():
result = {"changed": False}

name = module.params.get("name")
resource_type = module.params.get("resource_type")
state = module.params.get("state")

params = {}
Expand Down
10 changes: 2 additions & 8 deletions plugins/modules/data_pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,10 +312,7 @@ def check_dp_exists(client, dp_id):
"""
try:
# pipeline_description raises DataPipelineNotFound
if pipeline_description(client, dp_id):
return True
else:
return False
return bool(pipeline_description(client, dp_id))
except DataPipelineNotFound:
return False

Expand All @@ -331,10 +328,7 @@ def check_dp_status(client, dp_id, status):
"""
if not isinstance(status, list):
raise AssertionError()
if pipeline_field(client, dp_id, field="@pipelineState") in status:
return True
else:
return False
return bool(pipeline_field(client, dp_id, field="@pipelineState") in status)


def pipeline_status_timeout(client, dp_id, status, timeout):
Expand Down
5 changes: 1 addition & 4 deletions plugins/modules/directconnect_gateway.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,7 @@ def wait_for_status(client, module, gateway_id, virtual_gateway_id, status):
if response["directConnectGatewayAssociations"][0]["associationState"] == status:
status_achieved = True
break
else:
time.sleep(polling_increment_secs)
time.sleep(polling_increment_secs)
else:
status_achieved = True
break
Expand Down Expand Up @@ -304,7 +303,6 @@ def ensure_absent(client, module):
# then a match is considered to have been found and we will not create another dxgw.

changed = False
result = dict()
dx_gateway_id = module.params.get("direct_connect_gateway_id")
existing_dxgw = find_dx_gateway(client, module, dx_gateway_id)
if existing_dxgw is not None:
Expand Down Expand Up @@ -333,7 +331,6 @@ def ensure_absent(client, module):
)
except (botocore.exceptions.BotoCoreError, botocore.exceptions.ClientError) as e:
module.fail_json_aws(e, msg="Failed to delete gateway")
result = resp["directConnectGateway"]
return changed


Expand Down
2 changes: 1 addition & 1 deletion plugins/modules/directconnect_link_aggregation_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ def disassociate_vis(client, lag_id, virtual_interfaces):
for vi in virtual_interfaces:
delete_virtual_interface(client, vi["virtualInterfaceId"])
try:
response = client.delete_virtual_interface(virtualInterfaceId=vi["virtualInterfaceId"])
client.delete_virtual_interface(virtualInterfaceId=vi["virtualInterfaceId"])
except botocore.exceptions.ClientError as e:
raise DirectConnectError(
msg=f"Could not delete virtual interface {vi} to delete link aggregation group {lag_id}.",
Expand Down
10 changes: 5 additions & 5 deletions plugins/modules/dms_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -520,12 +520,12 @@ def compare_params(param_described):
param_described.pop("Tags", None)
modparams.pop("Tags", None)
changed = False
for paramname in modparams:
for param_name, param_value in modparams.items():
if (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can add better simplified this like:

if not (....):
    changed = True
    break

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this function as a whole, I'm going to leave it this way and rework + some unit tests. The logic's a real mess and can actually do a "return True" inside the loop

paramname == "Password"
or paramname in param_described
and param_described[paramname] == modparams[paramname]
or str(param_described[paramname]).lower() == modparams[paramname]
param_name == "Password"
or param_name in param_described
and param_described[param_name] == param_value
or str(param_described[param_name]).lower() == param_value
):
pass
else:
Expand Down
6 changes: 3 additions & 3 deletions plugins/modules/dms_replication_subnet_group.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,10 @@ def compare_params(module, param_described):
param_described.pop("VpcId")
if "SubnetGroupStatus" in param_described.keys():
param_described.pop("SubnetGroupStatus")
for paramname in modparams.keys():
if paramname in param_described.keys() and param_described.get(paramname) == modparams[paramname]:
for param_name, param_value in modparams.items():
if param_name in param_described and param_described.get(param_name) == param_value:
pass
elif paramname == "SubnetIds":
elif param_name == "SubnetIds":
subnets = []
for subnet in param_described.get("Subnets"):
subnets.append(subnet.get("SubnetIdentifier"))
Expand Down
Loading
Loading