From e13fc5d6bd6a632c08bf8c2bed09eb9066bbbcf7 Mon Sep 17 00:00:00 2001 From: Sloane Hertel Date: Tue, 2 May 2017 10:13:56 -0400 Subject: [PATCH] elasticache_parameter_group: fix documentation and exception handling - fixes #23709 (#23718) * fix documentation and correct exception handling * follow AWS exception guidelines * fix parameter_group_family req; only needed when creating cache parameter group make pep8 and remove from legacy files --- .../amazon/elasticache_parameter_group.py | 35 ++++++++++++------- test/sanity/pep8/legacy-files.txt | 1 - 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/lib/ansible/modules/cloud/amazon/elasticache_parameter_group.py b/lib/ansible/modules/cloud/amazon/elasticache_parameter_group.py index dd41972a1de..46a73ef3396 100644 --- a/lib/ansible/modules/cloud/amazon/elasticache_parameter_group.py +++ b/lib/ansible/modules/cloud/amazon/elasticache_parameter_group.py @@ -32,8 +32,9 @@ options: group_family: description: - The name of the cache parameter group family that the cache parameter group can be used with. + Required when creating a cache parameter group. choices: ['memcached1.4', 'redis2.6', 'redis2.8', 'redis3.2'] - required: yes + required: no name: description: - A user-specified name for the cache parameter group. @@ -48,7 +49,7 @@ options: required: true values: description: - - A user-specified list of parameters to reset or modify for the cache parameter group. + - A user-specified dictionary of parameters to reset or modify for the cache parameter group. required: no default: None """ @@ -70,8 +71,8 @@ EXAMPLES = """ elasticache_parameter_group: name: 'test-param-group' values: - - ['activerehashing', 'yes'] - - ['client-output-buffer-limit-normal-hard-limit', 4] + activerehashing: yes + client-output-buffer-limit-normal-hard-limit: 4 state: 'present' - name: 'Reset all modifiable parameters for the test parameter group' elasticache_parameter_group: @@ -123,25 +124,28 @@ try: except ImportError: HAS_BOTO3 = False + def create(module, conn, name, group_family, description): """ Create ElastiCache parameter group. """ try: response = conn.create_cache_parameter_group(CacheParameterGroupName=name, CacheParameterGroupFamily=group_family, Description=description) changed = True - except boto.exception.BotoServerError as e: - module.fail_json(msg="Unable to create cache parameter group.", exception=traceback.format_exc()) + except botocore.exceptions.ClientError as e: + module.fail_json(msg="Unable to create cache parameter group.", exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response)) return response, changed + def delete(module, conn, name): """ Delete ElastiCache parameter group. """ try: conn.delete_cache_parameter_group(CacheParameterGroupName=name) response = {} changed = True - except boto.exception.BotoServerError as e: - module.fail_json(msg="Unable to delete cache parameter group.", exception=traceback.format_exc()) + except botocore.exceptions.ClientError as e: + module.fail_json(msg="Unable to delete cache parameter group.", exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response)) return response, changed + def make_current_modifiable_param_dict(module, conn, name): """ Gets the current state of the cache parameter group and creates a dict with the format: {ParameterName: [Allowed_Values, DataType, ParameterValue]}""" current_info = get_info(conn, name) @@ -156,6 +160,7 @@ def make_current_modifiable_param_dict(module, conn, name): modifiable_params[param["ParameterName"]] = [param["AllowedValues"], param["DataType"], param["ParameterValue"]] return modifiable_params + def check_valid_modification(module, values, modifiable_params): """ Check if the parameters and values in values are valid. """ changed_with_update = False @@ -184,6 +189,7 @@ def check_valid_modification(module, values, modifiable_params): return changed_with_update + def check_changed_parameter_values(values, old_parameters, new_parameters): """ Checking if the new values are different than the old values. """ changed_with_update = False @@ -203,6 +209,7 @@ def check_changed_parameter_values(values, old_parameters, new_parameters): return changed_with_update + def modify(module, conn, name, values): """ Modify ElastiCache parameter group to reflect the new information if it differs from the current. """ # compares current group parameters with the parameters we've specified to to a value to see if this will change the group @@ -212,10 +219,11 @@ def modify(module, conn, name, values): format_parameters.append({'ParameterName': key, 'ParameterValue': value}) try: response = conn.modify_cache_parameter_group(CacheParameterGroupName=name, ParameterNameValues=format_parameters) - except boto.exception.BotoServerError as e: - module.fail_json(msg="Unable to modify cache parameter group.", exception=traceback.format_exc()) + except botocore.exceptions.ClientError as e: + module.fail_json(msg="Unable to modify cache parameter group.", exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response)) return response + def reset(module, conn, name, values): """ Reset ElastiCache parameter group if the current information is different from the new information. """ # used to compare with the reset parameters' dict to see if there have been changes @@ -235,8 +243,8 @@ def reset(module, conn, name, values): try: response = conn.reset_cache_parameter_group(CacheParameterGroupName=name, ParameterNameValues=format_parameters, ResetAllParameters=all_parameters) - except boto.exception.BotoServerError as e: - module.fail_json(msg="Unable to reset cache parameter group.", exception=traceback.format_exc()) + except botocore.exceptions.ClientError as e: + module.fail_json(msg="Unable to reset cache parameter group.", exception=traceback.format_exc(), **camel_dict_to_snake_dict(e.response)) # determine changed new_parameters_dict = make_current_modifiable_param_dict(module, conn, name) @@ -244,6 +252,7 @@ def reset(module, conn, name, values): return response, changed + def get_info(conn, name): """ Gets info about the ElastiCache parameter group. Returns false if it doesn't exist or we don't have access. """ try: @@ -287,7 +296,7 @@ def main(): exists = get_info(connection, parameter_group_name) # check that the needed requirements are available - if state == 'present' and not exists and not (parameter_group_family or group_description): + if state == 'present' and not (exists and parameter_group_family and group_description): module.fail_json(msg="Creating a group requires a family group and a description.") elif state == 'reset' and not exists: module.fail_json(msg="No group %s to reset. Please create the group before using the state 'reset'." % parameter_group_name) diff --git a/test/sanity/pep8/legacy-files.txt b/test/sanity/pep8/legacy-files.txt index f7f9af3144a..6da7723c54c 100644 --- a/test/sanity/pep8/legacy-files.txt +++ b/test/sanity/pep8/legacy-files.txt @@ -189,7 +189,6 @@ lib/ansible/modules/cloud/amazon/ecs_taskdefinition.py lib/ansible/modules/cloud/amazon/efs.py lib/ansible/modules/cloud/amazon/efs_facts.py lib/ansible/modules/cloud/amazon/elasticache.py -lib/ansible/modules/cloud/amazon/elasticache_parameter_group.py lib/ansible/modules/cloud/amazon/elasticache_snapshot.py lib/ansible/modules/cloud/amazon/elasticache_subnet_group.py lib/ansible/modules/cloud/amazon/execute_lambda.py