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

Add New Attributes #17

Merged
merged 11 commits into from
Dec 29, 2018
Merged

Add New Attributes #17

merged 11 commits into from
Dec 29, 2018

Conversation

johncblandii
Copy link
Contributor

@johncblandii johncblandii commented Dec 13, 2018

what

  • Added additional performance tuning parameters
  • Added basic example

why

  • New enhancements available from underlying terraform resource

@osterman osterman requested a review from aknysh December 13, 2018 23:15
outputs.tf Outdated Show resolved Hide resolved
outputs.tf Outdated Show resolved Hide resolved
Copy link
Member

@osterman osterman left a comment

Choose a reason for hiding this comment

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

Just (2) nitpicks. Then rebuild README.

osterman and others added 3 commits December 13, 2018 17:43
Co-Authored-By: johncblandii <johncblandii@users.noreply.github.com>
Co-Authored-By: johncblandii <johncblandii@users.noreply.github.com>
example/main.tf Outdated Show resolved Hide resolved
outputs.tf Outdated Show resolved Hide resolved
example/variables.tf Outdated Show resolved Hide resolved
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

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

thanks @johncblandii
looks good, just a few comments

@aknysh aknysh requested a review from osterman December 14, 2018 02:14
@johncblandii
Copy link
Contributor Author

Ready for a final review.

Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

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

thanks @johncblandii
for the final touch, please run terraform fmt, TravisCI is failing
https://travis-ci.org/cloudposse/terraform-aws-efs/builds/467832858

osterman
osterman previously approved these changes Dec 28, 2018
@osterman osterman requested a review from aknysh December 28, 2018 17:35
@osterman osterman changed the title Updated attributes to the latest Add New Attributes Dec 28, 2018
outputs.tf Outdated
description = "DNS name"
}

output "mount_target_dns_names" {
value = "${aws_efs_mount_target.default.*.dns_name}"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
value = "${aws_efs_mount_target.default.*.dns_name}"
value = "[${aws_efs_mount_target.default.*.dns_name}"]

Copy link
Member

Choose a reason for hiding this comment

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

@johncblandii sorry, I missed this.
Since we use [] in all other outputs (e.g. network_interface_ids), let's make it consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

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

see comments

Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

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

thanks @johncblandii

@aknysh aknysh merged commit ed71ef5 into cloudposse:master Dec 29, 2018
@johncblandii johncblandii deleted the attr_updates branch December 29, 2018 16:30
netflash referenced this pull request in netflash/terraform-aws-efs Apr 25, 2020
…ssh_keys_to_workers

Allow adding SSH keys to worker ASGs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants