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

Aurora rds #6570

Closed
wants to merge 2 commits into from
Closed

Aurora rds #6570

wants to merge 2 commits into from

Conversation

arcadiatea
Copy link
Contributor

This is pull request to add aws rds db cluster parameter group function.
[GH-6568]

@jen20
Copy link
Contributor

jen20 commented May 11, 2016

Hi @arcadiatea! Thanks for opening a pull request here! The actual resource added here looks great, but it seems we picked up a lot of stray line ending changes in unrelated dependencies, and a changelog entry which is traditionally made on master. Is there any chance you could adjust the commit to just contain the necessary changes to the AWS provider and force push it back to this branch so that we can merge? Thanks!

@arcadiatea
Copy link
Contributor Author

I reverted the unrelated dependency change, most of them are result of gofmt.

@stack72
Copy link
Contributor

stack72 commented May 13, 2016

Hi @arcadiatea

Please can you also remove the changelog differences as well? We usually add the changelog entry after merge to make sure there are no issues

Thanks

Paul

@arcadiatea
Copy link
Contributor Author

I cleaned up the changes. How it looks now?

@stack72
Copy link
Contributor

stack72 commented May 13, 2016

Hi @arcadiatea

The changes look good now. I just ran the tests to make sure all was well:

make testacc TEST=./builtin/providers/aws TESTARGS='-run=TestAccAWSDBClusterParameterGroup' 2>~/tf.log
==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /vendor/)
TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSDBClusterParameterGroup -timeout 120m
=== RUN   TestAccAWSDBClusterParameterGroup_basic
--- FAIL: TestAccAWSDBClusterParameterGroup_basic (8.58s)
    testing.go:234: Step 0 error: Error applying: 1 error(s) occurred:

        * aws_db_cluster_parameter_group.bar: Error creating DB Cluster Parameter Group: InvalidParameterValue: ParameterGroupFamily mysql5.6 does not support DB clusters
            status code: 400, request id: 12107477-1938-11e6-b038-b17e58d2ad7d
=== RUN   TestAccAWSDBClusterParameterGroup_Only
--- FAIL: TestAccAWSDBClusterParameterGroup_Only (9.18s)
    testing.go:234: Step 0 error: Error applying: 1 error(s) occurred:

        * aws_db_cluster_parameter_group.bar: Error creating DB Cluster Parameter Group: InvalidParameterValue: ParameterGroupFamily mysql5.6 does not support DB clusters
            status code: 400, request id: 17831bd9-1938-11e6-b038-b17e58d2ad7d
FAIL
exit status 1
FAIL    github.com/hashicorp/terraform/builtin/providers/aws    17.792s

Please can you have a look at this?

Paul

@arcadiatea
Copy link
Contributor Author

Hi Paul
Thank you for testing. The db cluster parameter group is special for Aurora only.
So the family group need to be "aurora5.6".

Here is the example

resource "aws_db_cluster_parameter_group" "default" {
name = "test"
family = "aurora5.6"

parameter {
name = "character_set_database"
value = "utf8"
apply_method = "immediate"
}
}

@stack72
Copy link
Contributor

stack72 commented May 16, 2016

Hi @arcadiatea

Thanks for clarifying here. Please can you update the PR with a test that works? Once we get this in place, we can proceed with the merge :)

P.

@arcadiatea
Copy link
Contributor Author

Hi, @stack72
The test code is updated.
Thanks
Linda

@arcadiatea
Copy link
Contributor Author

@stack72 do you have chance to take look this?

@catsby
Copy link
Contributor

catsby commented Jun 2, 2016

Hello – I believe this was already added in #5269, pinging @stack72 who's looked at both to confirm

@catsby
Copy link
Contributor

catsby commented Jun 2, 2016

I chatted with Paul and this does look like mistaken duplicate work, so sorry! I think #6865 is still good though. Sorry for the trouble

@catsby catsby closed this Jun 2, 2016
@arcadiatea arcadiatea deleted the aurora_rds branch January 26, 2017 07:51
@ghost
Copy link

ghost commented Apr 17, 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 17, 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.

4 participants