r/aws_dynamodb_table: Ensure ttl is properly read #2452

bflad · 2017-11-28T08:14:04Z

Previously it was pointing to a non-existent timeToLive attribute and not returning any errors.

make testacc TEST=./aws TESTARGS='-run=TestAccAWSDynamoDbTable'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSDynamoDbTable -timeout 120m
=== RUN   TestAccAWSDynamoDbTable_importBasic
--- PASS: TestAccAWSDynamoDbTable_importBasic (63.75s)
=== RUN   TestAccAWSDynamoDbTable_importTags
--- PASS: TestAccAWSDynamoDbTable_importTags (61.60s)
=== RUN   TestAccAWSDynamoDbTable_importTimeToLive
--- PASS: TestAccAWSDynamoDbTable_importTimeToLive (56.04s)
=== RUN   TestAccAWSDynamoDbTable_basic
--- PASS: TestAccAWSDynamoDbTable_basic (324.05s)
=== RUN   TestAccAWSDynamoDbTable_streamSpecification
--- PASS: TestAccAWSDynamoDbTable_streamSpecification (51.13s)
=== RUN   TestAccAWSDynamoDbTable_tags
--- PASS: TestAccAWSDynamoDbTable_tags (56.07s)
=== RUN   TestAccAWSDynamoDbTable_gsiUpdate
--- PASS: TestAccAWSDynamoDbTable_gsiUpdate (195.70s)
=== RUN   TestAccAWSDynamoDbTable_ttl
--- PASS: TestAccAWSDynamoDbTable_ttl (71.26s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	879.650s

radeksimko

Just 1 question + one suggestion.

radeksimko · 2017-12-11T09:55:59Z

aws/resource_aws_dynamodb_table.go

+			"attribute_name": *timeToLiveOutput.TimeToLiveDescription.AttributeName,
+			"enabled":        (*timeToLiveOutput.TimeToLiveDescription.TimeToLiveStatus == dynamodb.TimeToLiveStatusEnabled),
+		}
+		timeToLiveList = append(timeToLiveList, timeToLiveSet)


Nitpick: As we know we're only ever going to need 1 element of the slice we can make it a fixed-size array:

timeToLiveList := [...]interface{}{ map[string]interface{}{ "attribute_name": *timeToLiveOutput.TimeToLiveDescription.AttributeName, "enabled": (*timeToLiveOutput.TimeToLiveDescription.TimeToLiveStatus == dynamodb.TimeToLiveStatusEnabled), }, }

radeksimko · 2017-12-11T12:28:52Z

aws/resource_aws_dynamodb_table.go


-	log.Printf("[DEBUG] Loaded TimeToLive data for DynamoDB table '%s'", d.Id())
+	if timeToLiveOutput.TimeToLiveDescription.AttributeName != nil {


Any reason we're checking timeToLiveOutput.TimeToLiveDescription.AttributeName instead of timeToLiveOutput.TimeToLiveDescription here?

I guess we can assume AttributeName is not nil if the parent struct is not nil?

We cannot actually. 😄 It causes a panic on AttributeName dereference in the basic acceptance testing without the additional nil check. I will update it to check both the parent struct and AttributeName

panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x2537d6f] goroutine 375 [running]: github.com/terraform-providers/terraform-provider-aws/aws.flattenAwsDynamoDbTableResource(0xc42019b650, 0x3217440, 0xc420532480, 0xc4207e8160, 0x0, 0x5168700) /Users/bflad/go/src/github.com/terraform-providers/terraform-provider-aws/aws/resource_aws_dynamodb_table.go:823 +0x1a1f

* Add timeToLiveOutput.TimeToLiveDescription nil check * Simplify logic to d.Set ttl

bflad · 2017-12-11T21:55:05Z

@radeksimko updated!

Add timeToLiveOutput.TimeToLiveDescription nil check
Simplify logic to d.Set ttl

make testacc TEST=./aws TESTARGS='-run=TestAccAWSDynamoDbTable'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSDynamoDbTable -timeout 120m
=== RUN   TestAccAWSDynamoDbTable_importBasic
--- PASS: TestAccAWSDynamoDbTable_importBasic (58.48s)
=== RUN   TestAccAWSDynamoDbTable_importTags
--- PASS: TestAccAWSDynamoDbTable_importTags (73.62s)
=== RUN   TestAccAWSDynamoDbTable_importTimeToLive
--- PASS: TestAccAWSDynamoDbTable_importTimeToLive (63.36s)
=== RUN   TestAccAWSDynamoDbTable_basic
--- PASS: TestAccAWSDynamoDbTable_basic (387.61s)
=== RUN   TestAccAWSDynamoDbTable_streamSpecification
--- PASS: TestAccAWSDynamoDbTable_streamSpecification (81.41s)
=== RUN   TestAccAWSDynamoDbTable_tags
--- PASS: TestAccAWSDynamoDbTable_tags (59.59s)
=== RUN   TestAccAWSDynamoDbTable_gsiUpdate
--- PASS: TestAccAWSDynamoDbTable_gsiUpdate (188.32s)
=== RUN   TestAccAWSDynamoDbTable_ttl
--- PASS: TestAccAWSDynamoDbTable_ttl (53.18s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	965.599s

radeksimko · 2017-12-12T09:57:58Z

aws/resource_aws_dynamodb_table.go


-	log.Printf("[DEBUG] Loaded TimeToLive data for DynamoDB table '%s'", d.Id())
+	if timeToLiveOutput.TimeToLiveDescription != nil && timeToLiveOutput.TimeToLiveDescription.AttributeName != nil {
+		timeToLiveList := []interface{}{


This is certainly more compact, but it's still a slice, not an array. The three dots I mentioned in my example were turning it into an array.

https://blog.golang.org/go-slices-usage-and-internals

It's certainly not a big deal though.

* Add Data Source: aws_elb * Fix dataSourceAwsElb typo * Fix dataSourceAwsElb Schema name field to not include Computed * Remove dataSourceAwsElb schema defaults for computed fields * Remove dataSourceAwsElb schema defaults for nested computed fields too * Corrected depends_on entry for EIP depends_on = ["aws_internet_gateway.gw"] is the correct syntax * Add sweeper for IAM Server Certificates * test/aws_config_delivery_channel: Add missing dependencies * d/aws_elb r/aws_elb: hashicorp#2004 review comments * Remove enable_deletion_protection from testAccDataSourceAWSELBConfigBasic * Replace unnecessary errwrap.Wrapf with fmt.Errorf * Reduce flattenAwsELbResource to ec2conn and elbconn instead of meta * Properly name TestAccDataSourceAWSELB_basic resources * Use t.Name() for description and TestName tags * d/aws_elb: Fix documentation sidebar ordering after merging master with new d/aws_elasticache_replication_group * Makefile: Add sweep target * Update cognito_user_pool.markdown * Update CHANGELOG.md * r/aws_elasticache_security_group: add import support (hashicorp#2277) * r/aws_elasticache_security_group: add import support * r/aws_elasticache_security_group: hashicorp#2277 review updates * Use d.Id() instead of d.Get("name") on read, which allows using schema.ImportStatePassthrough * d.Set("security_group_names") on read * Set AWS_DEFAULT_REGION to us-east-1 on import testing * Update CHANGELOG.md * documentation: remove antislashes in page titles * Added missing WARN debug lines when reading a non-existing resource * Removed <wbr> from documentation titles * vendor: Bump aws-sdk-go to v.1.12.44 * Add logs for iam server certificate delete conflict (hashicorp#2533) * Query elb API for load balancer arn causing delete conflict - For IAM server certificate. * Use regex for lb name. * Edits for hashicorp#2533 * r/aws_sqs_queue_policy: Support import by queue URL (hashicorp#2544) * Update CHANGELOG.md * r/aws_elasticsearch_domain: Add LogPublishingOption (hashicorp#2285) * WIP * Add enabled * Use cwl policy * Reflect reviews * Update CHANGELOG.md * Add force_destroy field to aws_athena_database (hashicorp#2363) * Add force_destroy field to aws_athena_database. Fixes hashicorp#2362. * Remove unnecessary import. * Code review feedback * Update CHANGELOG.md * Add more example and missing field * New Resource: aws_media_store_container (hashicorp#2448) * New Resource: aws_media_store_container * Reflect reviews * remove policy * Update CHANGELOG.md * Add Redis AUTH, in-transit and at-rest encryption (hashicorp#2090) * add AUTH, at-rest and in-transit encryption to Elasticache replication groups * add _enabled to transit/at_rest encyrption parameters * added one more _enabled * move validateAwsElastiCacheReplicationGroupAuthToken to aws/validators.go, as well as tests * set auth_token to nil during Reads * update Replication Group encryption acceptance tests to use config functions instead of vars * Fix whitespacing (tabs -> spaces) * docs/elasticache_replication_group: Add missing fields * Update CHANGELOG.md * r/aws_dynamodb_table: Ensure ttl is properly read (hashicorp#2452) * r/aws_dynamodb_table: Ensure ttl is properly read * r/aws_dynamodb_table: hashicorp#2452 review updates * Add timeToLiveOutput.TimeToLiveDescription nil check * Simplify logic to d.Set ttl * Update CHANGELOG.md * Bump aws-sdk-go to v.1.12.45 * New Resource: PublicDnsNamespace (hashicorp#2569) * WIP * Add test, docs * Reflect reviews * Modify error handling * Update CHANGELOG.md * New Resource: ServiceDiscovery PrivateDNS Namespace (hashicorp#2589) * New Resource: service_discovery_private_dns_namespace * Reflect reviews * Update CHANGELOG.md

* Makefile: Add sweep target * Add more example and missing field * docs/elasticache_replication_group: Add missing fields * Update CHANGELOG.md * r/aws_dynamodb_table: Ensure ttl is properly read (hashicorp#2452) * r/aws_dynamodb_table: Ensure ttl is properly read * r/aws_dynamodb_table: hashicorp#2452 review updates * Add timeToLiveOutput.TimeToLiveDescription nil check * Simplify logic to d.Set ttl * Update CHANGELOG.md * Bump aws-sdk-go to v.1.12.45 * New Resource: PublicDnsNamespace (hashicorp#2569) * WIP * Add test, docs * Reflect reviews * Modify error handling * Update CHANGELOG.md * New Resource: ServiceDiscovery PrivateDNS Namespace (hashicorp#2589) * New Resource: service_discovery_private_dns_namespace * Reflect reviews * Update CHANGELOG.md * hashicorp#2217: re-enable default encryption after disabling in via UI * hashicorp#2217: documentation

ghost · 2020-04-10T17:20:34Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

r/aws_dynamodb_table: Ensure ttl is properly read

f9a88c4

bflad mentioned this pull request Nov 28, 2017

DynamoDB ttl refresh #2442

Closed

radeksimko added the bug Addresses a defect in current functionality. label Nov 30, 2017

radeksimko reviewed Dec 11, 2017

View reviewed changes

radeksimko added the waiting-response Maintainers are waiting on response from community or contributor. label Dec 11, 2017

r/aws_dynamodb_table: hashicorp#2452 review updates

394b67e

* Add timeToLiveOutput.TimeToLiveDescription nil check * Simplify logic to d.Set ttl

radeksimko reviewed Dec 12, 2017

View reviewed changes

radeksimko merged commit edb26df into hashicorp:master Dec 12, 2017

bflad mentioned this pull request Feb 1, 2018

DynamoDB Import does not import TTL settings #3235

Closed

ghost locked and limited conversation to collaborators Apr 10, 2020

breathingdust removed the waiting-response Maintainers are waiting on response from community or contributor. label Sep 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

r/aws_dynamodb_table: Ensure ttl is properly read #2452

r/aws_dynamodb_table: Ensure ttl is properly read #2452

bflad commented Nov 28, 2017

radeksimko left a comment

radeksimko Dec 11, 2017

radeksimko Dec 11, 2017

bflad Dec 11, 2017

bflad commented Dec 11, 2017

radeksimko Dec 12, 2017

ghost commented Apr 10, 2020


		log.Printf("[DEBUG] Loaded TimeToLive data for DynamoDB table '%s'", d.Id())
		if timeToLiveOutput.TimeToLiveDescription.AttributeName != nil {

r/aws_dynamodb_table: Ensure ttl is properly read #2452

r/aws_dynamodb_table: Ensure ttl is properly read #2452

Conversation

bflad commented Nov 28, 2017

radeksimko left a comment

Choose a reason for hiding this comment

radeksimko Dec 11, 2017

Choose a reason for hiding this comment

radeksimko Dec 11, 2017

Choose a reason for hiding this comment

bflad Dec 11, 2017

Choose a reason for hiding this comment

bflad commented Dec 11, 2017

radeksimko Dec 12, 2017

Choose a reason for hiding this comment

ghost commented Apr 10, 2020