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

provider/aws: Enable snapshotting by default on aws_redshift_cluster #11695

Merged
merged 1 commit into from
Feb 15, 2017

Conversation

stack72
Copy link
Contributor

@stack72 stack72 commented Feb 4, 2017

This extends the work in #11668 to enable final snapshots by default.
This time it's for redshift

% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSRedshiftCluster_withFinalSnapshot'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/02/04 13:53:02 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSRedshiftCluster_withFinalSnapshot -timeout 120m
=== RUN   TestAccAWSRedshiftCluster_withFinalSnapshot
--- PASS: TestAccAWSRedshiftCluster_withFinalSnapshot (859.96s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	859.986s

Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

Besides the randomization this is looking 👌

}
}

snapshot_identifier := "tf-acctest-snapshot-1"
Copy link
Member

Choose a reason for hiding this comment

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

Similarly to RDS snapshots I think we should randomize the snapshot IDs here.

@stack72 stack72 force-pushed the f-aws-redshift-snapshot-default branch 4 times, most recently from 960b621 to 0b93f18 Compare February 15, 2017 15:24
This extends the work in #11668 to enable final snapshots by default.
This time it's for redshift

```
% make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSRedshiftCluster_withFinalSnapshot'
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/02/04 13:53:02 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSRedshiftCluster_withFinalSnapshot -timeout 120m
=== RUN   TestAccAWSRedshiftCluster_withFinalSnapshot
--- PASS: TestAccAWSRedshiftCluster_withFinalSnapshot (859.96s)
PASS
ok  	github.com/hashicorp/terraform/builtin/providers/aws	859.986s
```
@stack72 stack72 force-pushed the f-aws-redshift-snapshot-default branch from 0b93f18 to 87add09 Compare February 15, 2017 16:22
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

Assuming acceptance tests are passing, this LGTM.

@stack72 stack72 merged commit da22022 into master Feb 15, 2017
@stack72 stack72 deleted the f-aws-redshift-snapshot-default branch February 15, 2017 16:37
minamijoyo added a commit to minamijoyo/terraform-provider-aws that referenced this pull request Jul 11, 2017
The default value of `skip_final_snapshot` was changed to `false`
with the following change, but the document was not updated.
hashicorp/terraform#11695
stack72 pushed a commit to hashicorp/terraform-provider-aws that referenced this pull request Jul 11, 2017
…1121)

The default value of `skip_final_snapshot` was changed to `false`
with the following change, but the document was not updated.
hashicorp/terraform#11695
@ghost
Copy link

ghost commented Apr 16, 2020

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants