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: Update VPC Peering connect accept/request attributes (supersedes #8338) #8432

Merged
merged 6 commits into from
Aug 24, 2016

Conversation

catsby
Copy link
Contributor

@catsby catsby commented Aug 23, 2016

This PR continues #8338 by reading the peering options regardless if present.
I'm currently seeing a diff on accepter though so this is a WIP

@kwilczynski
Copy link
Contributor

@catsby this is how my original code looked like. The constant change to accepter is what prompted me to apply the approach from #8338.

@catsby
Copy link
Contributor Author

catsby commented Aug 24, 2016

@kwilczynski I think using TypeSet is better here, and removes this issue we were hitting:

@catsby catsby changed the title [WIP] provider/aws: Update VPC Peering connect accept/request attributes (supersedes #8338) provider/aws: Update VPC Peering connect accept/request attributes (supersedes #8338) Aug 24, 2016
@@ -67,7 +67,7 @@ func resourceAwsVPCPeeringCreate(d *schema.ResourceData, meta interface{}) error

resp, err := conn.CreateVpcPeeringConnection(createOpts)
if err != nil {
return fmt.Errorf("Error creating vpc peering connection: %s", err)
return fmt.Errorf("Error creating VPC Peering Connection: %s", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably want to tend towards errwrap.Wrapf("Error creating VPC Peering Connection: {{err}}", err) here

@catsby
Copy link
Contributor Author

catsby commented Aug 24, 2016

Updated to fix a panic, changing the nested accepter and requester attributes to have defaults matching the API instead of being optional, computed

TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSRouteTable_vpcPeering -timeout 120m
=== RUN   TestAccAWSRouteTable_vpcPeering
--- PASS: TestAccAWSRouteTable_vpcPeering (31.36s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    31.387s

TF_ACC=1 go test ./builtin/providers/aws -v -run=TestAccAWSVPCPeeringConnection -timeout 120m
=== RUN   TestAccAWSVPCPeeringConnection_importBasic
--- PASS: TestAccAWSVPCPeeringConnection_importBasic (18.29s)
=== RUN   TestAccAWSVPCPeeringConnection_basic
--- PASS: TestAccAWSVPCPeeringConnection_basic (18.37s)
=== RUN   TestAccAWSVPCPeeringConnection_plan
--- PASS: TestAccAWSVPCPeeringConnection_plan (19.48s)
=== RUN   TestAccAWSVPCPeeringConnection_tags
--- PASS: TestAccAWSVPCPeeringConnection_tags (18.36s)
=== RUN   TestAccAWSVPCPeeringConnection_options
--- PASS: TestAccAWSVPCPeeringConnection_options (32.88s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/aws    107.415s

@jen20
Copy link
Contributor

jen20 commented Aug 24, 2016

LGTM, I think we can consider the acceptor -> accepter thing later.

@kwilczynski
Copy link
Contributor

@jen20 I believe that "accepter" is not a typo.

"acceptor", a noun, meaning "one who accepts";
"accepter", a noun, meaning "a person who accepts".

  1. http://www.homophone.com/h/accepter-acceptor
  2. http://www.merriam-webster.com/dictionary/accepter
  3. http://www.merriam-webster.com/dictionary/acceptor
  4. http://docs.aws.amazon.com/AmazonVPC/latest/PeeringGuide/working-with-vpc-peering.html

The latter is more common in American English, IIRC.

Regardless of the spelling, the AWS documentation and the API spell it as "accepter", and there is a prerogative to follow the API/documentation as closely as possible, which is what I did.

richardbowden pushed a commit to richardbowden/terraform that referenced this pull request Aug 27, 2016
…upersedes hashicorp#8338) (hashicorp#8432)

* Fix crash when reading VPC Peering Connection options.

This resolves the issue introduced in hashicorp#8310.

Signed-off-by: Krzysztof Wilczynski <[email protected]>

* Do not de-reference values when using Set().

Signed-off-by: Krzysztof Wilczynski <[email protected]>

* provider/aws: Update VPC Peering connect accept/request attributes

* change from type list to type set

* provider/aws: Update VPC Peering accept/requst options, tests

* errwrap some things
@ghost
Copy link

ghost commented Apr 22, 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 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants