Various pylint fixups (#2221)

SUMMARY

simplifiable-if-statement  (use return bool(...) rather than if (...); return True ; else ; return False
consider-using-dict-items
redefined-builtin (using id and input as variables)
no-else-break
redefined-outer-name  (mostly fixtures)
pointless-string-statement  (Strings being used as comments, function documentation before function rather than as first line)
Remove unused variable assignments

ISSUE TYPE

Feature Pull Request

COMPONENT NAME
plugins/connection/aws_ssm.py
plugins/module_utils/base.py
plugins/module_utils/ec2.py
plugins/module_utils/networkfirewall.py
plugins/modules/application_autoscaling_policy.py
plugins/modules/autoscaling_launch_config.py
plugins/modules/autoscaling_lifecycle_hook.py
plugins/modules/autoscaling_policy.py
plugins/modules/batch_job_definition.py
plugins/modules/cloudfront_distribution.py
plugins/modules/cloudfront_response_headers_policy.py
plugins/modules/codebuild_project.py
plugins/modules/config_aggregation_authorization.py
plugins/modules/config_delivery_channel.py
plugins/modules/config_recorder.py
plugins/modules/config_rule.py
plugins/modules/data_pipeline.py
plugins/modules/directconnect_gateway.py
plugins/modules/directconnect_link_aggregation_group.py
plugins/modules/dms_endpoint.py
plugins/modules/dms_replication_subnet_group.py
plugins/modules/dynamodb_table.py
plugins/modules/dynamodb_ttl.py
plugins/modules/ec2_carrier_gateway.py
plugins/modules/ec2_win_password.py
plugins/modules/ecs_ecr.py
plugins/modules/ecs_service_info.py
plugins/modules/ecs_taskdefinition.py
plugins/modules/efs.py
plugins/modules/efs_info.py
plugins/modules/eks_cluster.py
plugins/modules/elasticache.py
plugins/modules/elasticache_parameter_group.py
plugins/modules/elasticbeanstalk_app.py
plugins/modules/elb_target.py
plugins/modules/elb_target_group.py
plugins/modules/iam_server_certificate.py
plugins/modules/kinesis_stream.py
plugins/modules/lightsail_snapshot.py
plugins/modules/mq_broker.py
plugins/modules/mq_user.py
plugins/modules/mq_user_info.py
plugins/modules/msk_cluster.py
plugins/modules/networkfirewall_policy.py
plugins/modules/opensearch_info.py
plugins/modules/route53_wait.py
plugins/modules/s3_cors.py
plugins/modules/s3_lifecycle.py
plugins/modules/s3_logging.py
plugins/modules/s3_website.py
plugins/modules/secretsmanager_secret.py
plugins/modules/ses_identity.py
plugins/modules/sqs_queue.py
plugins/modules/storagegateway_info.py
plugins/modules/waf_condition.py
plugins/modules/waf_rule.py
plugins/modules/wafv2_ip_set.py
plugins/modules/wafv2_ip_set_info.py
plugins/modules/wafv2_resources.py
plugins/modules/wafv2_resources_info.py
plugins/modules/wafv2_rule_group.py
plugins/modules/wafv2_rule_group_info.py
plugins/modules/wafv2_web_acl.py
plugins/modules/wafv2_web_acl_info.py
ADDITIONAL INFORMATION

Reviewed-by: Bikouo Aubin
Reviewed-by: Alina Buzachis
Reviewed-by: Mark Chappell

changelogs/fragments/20250127-pylint.yml

@@ -0,0 +1,4 @@
+---
+minor_changes:
+  - various modules - linting fixups (https://github.com/ansible-collections/community.aws/pull/2221).
+  - dms_endpoint - improve resilience of parameter comparison (https://github.com/ansible-collections/community.aws/pull/2221).

plugins/connection/aws_ssm.py

@@ -299,7 +299,7 @@
 try:
     import boto3
     from botocore.client import Config
-except ImportError as e:
+except ImportError:
     pass
 
 from functools import wraps

plugins/module_utils/base.py

@@ -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())

plugins/module_utils/ec2.py

@@ -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:
@@ -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):

plugins/module_utils/networkfirewall.py

@@ -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
 
@@ -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
 

plugins/modules/application_autoscaling_policy.py

@@ -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"])

plugins/modules/autoscaling_launch_config.py

@@ -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")
@@ -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:

plugins/modules/autoscaling_lifecycle_hook.py

@@ -296,8 +296,6 @@ def main():
 
     connection = module.client("autoscaling")
 
-    changed = False
-
     if state == "present":
         create_lifecycle_hook(connection, module)
     elif state == "absent":

plugins/modules/autoscaling_policy.py

@@ -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:

plugins/modules/batch_job_definition.py

@@ -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:

plugins/modules/cloudfront_distribution.py

@@ -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:
@@ -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."
                 )

plugins/modules/cloudfront_response_headers_policy.py

@@ -163,19 +163,14 @@ def __init__(self, module):
     def find_response_headers_policy(self, name):
         try:
             policies = self.client.list_response_headers_policies()["ResponseHeadersPolicyList"]["Items"]
-
             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
+                    return self.client.get_response_headers_policy(Id=policy["ResponseHeadersPolicy"]["Id"])
 
-            return matching_policy
         except (ClientError, BotoCoreError) as e:
             self.module.fail_json_aws(e, msg="Error fetching policy information")
+        return None
 
     def create_response_header_policy(self, name, comment, cors_config, security_headers_config, custom_headers_config):
         cors_config = snake_dict_to_camel_dict(cors_config, capitalize_first=True)

plugins/modules/codebuild_project.py

@@ -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)
@@ -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

plugins/modules/config_aggregation_authorization.py

@@ -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"],
         )
@@ -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"],
             )
@@ -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"],
         )

plugins/modules/config_delivery_channel.py

@@ -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:

plugins/modules/config_recorder.py

@@ -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
@@ -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
@@ -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:

plugins/modules/config_rule.py

@@ -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,
         )
@@ -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 = {}

plugins/modules/data_pipeline.py

@@ -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
 
@@ -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):

plugins/modules/directconnect_gateway.py

@@ -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
@@ -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:
@@ -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
 
 

plugins/modules/directconnect_link_aggregation_group.py

@@ -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}.",

plugins/modules/dms_endpoint.py

@@ -516,21 +516,17 @@ def compare_params(param_described):
     """
     param_described = dict(param_described)
     modparams = create_module_params()
-    # modify can't update tags
-    param_described.pop("Tags", None)
-    modparams.pop("Tags", None)
-    changed = False
-    for paramname in modparams:
-        if (
-            paramname == "Password"
-            or paramname in param_described
-            and param_described[paramname] == modparams[paramname]
-            or str(param_described[paramname]).lower() == modparams[paramname]
-        ):
-            pass
-        else:
-            changed = True
-    return changed
+
+    # Note: this currently isn't as relaxed for the nested settings, (eg. S3Settings)
+    for param_name, param_value in modparams.items():
+        # modify can't update tags or password
+        if param_name in ["Tags", "Password"]:
+            continue
+        if param_name not in param_described:
+            return True
+        if param_described[param_name] != param_value and str(param_described[param_name]).lower() != param_value:
+            return True
+    return False
 
 
 def modify_dms_endpoint(connection, endpoint):