-
Notifications
You must be signed in to change notification settings - Fork 9.6k
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: AWS Redshift #3862
Conversation
eb47e52
to
dce61d8
Compare
80a5816
to
92fe91b
Compare
@@ -426,7 +427,36 @@ func TestExpandParameters(t *testing.T) { | |||
} | |||
} | |||
|
|||
<<<<<<< HEAD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some unresolved conflicts here I think?
92fe91b
to
c23b5f6
Compare
@radeksimko apologies for this - this has now been rectified |
2dd2a64
to
1493aaf
Compare
Just wondering if this is still "alive," or you need a hand? |
Finalising the schema and acceptance tests for the Redshift Security Group's
Creation of the schema, CRUD and acceptance tests for Redshift Parameter Group
Changed the aws_redshift_security_group and aws_redshift_parameter_group to remove the tags from the schema. Tags are a little bit more complicated than originally though - I will revisit this later Then added the schema, CRUD functionality and basic acceptance tests for aws_redshift_subnet_group Adding an acceptance test for the Update of subnet_ids in AWS Redshift Subnet Group
also removed the notion of tags from the redshift security group and parameter group documentation until that has been implemented Redshift Cluster CRUD and acceptance tests Removing the Acceptance test for the Cluster Updates. You cannot delete a cluster immediately after performing an operation on it. We would need to add a lot of retry logic to the system to get this test to work Adding some schema validation for RedShift cluster Adding the last of the pieces of a first draft of the Redshift work - this is the documentation
1493aaf
to
bf03752
Compare
@jonbca this will be merged very soon |
@stack72 all ready for review then? |
if awsErr, ok := err.(awserr.Error); ok { | ||
if "ClusterNotFound" == awsErr.Code() { | ||
d.SetId("") | ||
log.Printf("[DEBUG] Redshift Cluster (%s) not found", d.Id()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need this log statement before you call d.SetId("")
, otherwise you'll log an ""
😄
Wow, a lot of work done here, very nice @stack72! I did a review and things look good. The tests pass, pending a few tweaks (I'll link the commit in a follow up). Going to pull this in, thanks! |
Touchups: 8181a4e |
* master: (721 commits) Update CHANGELOG.md Update CHANGELOG.md Update CHANGELOG.md Handle external state changes for Packet resources gracefully. provider/aws: Add profile to provider config Bug fixes for aws_autoscaling_schedule resource Update CHANGELOG.md provider/docker: Fix flaky integration tests Update CHANGELOG.md minor clean ups after #3862 Update CHANGELOG.md Update CHANGELOG.md Update CHANGELOG.md provider/google: Content field for bucket objects provider/google: SQL user resource, documentation & tests Update CHANGELOG.md Update CHANGELOG.md Update CHANGELOG.md Update CHANGELOG.md move the 'color' assignment ...
…h_compute_environment-sweeper resource/aws_batch_*: Add test sweepers
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. |
The first version of this PR does not support tags. The following will be supported:
Redshift Cluster
Redshift Subnet Groups
Redshift Parameter Groups
Please note - there is no ability to apply parameter modifications immediately. Redshift needs a reboot to handle this
Redshift Security Groups
This is going to be a longer running PR as there is a lot of functionality in place. I may break this down into smaller PRs to make it easier to review - that can happen later