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.

Sign up for GitHub

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

r/aws_ssm_maintenance_window_target: Change MaxItems of targets #2361

Merged
merged 4 commits into from
Nov 21, 2017
Merged

r/aws_ssm_maintenance_window_target: Change MaxItems of targets #2361

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
aac1dba
WIP
atsushi-ishibashi Nov 18, 2017
776f37c
Keep required
atsushi-ishibashi Nov 20, 2017
ffe843b
Reflect reviews
atsushi-ishibashi Nov 21, 2017
fe65d60
Added checks in acceptance test
Ninir Nov 21, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion aws/resource_aws_ssm_maintenance_window_target.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func resourceAwsSsmMaintenanceWindowTarget() *schema.Resource {
Type: schema.TypeList,
Required: true,
ForceNew: true,
MaxItems: 1,
MaxItems: 5,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"key": {
Expand Down
4 changes: 4 additions & 0 deletions aws/resource_aws_ssm_maintenance_window_target_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ resource "aws_ssm_maintenance_window_target" "target" {
key = "tag:Name"
values = ["acceptance_test"]
}
targets {
key = "tag:Name2"
values = ["acceptance_test", "acceptance_test2"]
}
}
`, rName)
}
14 changes: 7 additions & 7 deletions aws/structure.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2661,7 +2661,7 @@ func sliceContainsMap(l []interface{}, m map[string]interface{}) (int, bool) {
}

func expandAwsSsmTargets(d *schema.ResourceData) []*ssm.Target {
var targets []*ssm.Target
targets := make([]*ssm.Target, 0)

targetConfig := d.Get("targets").([]interface{})

Expand All @@ -2685,13 +2685,13 @@ func flattenAwsSsmTargets(targets []*ssm.Target) []map[string]interface{} {
}

result := make([]map[string]interface{}, 0, len(targets))
target := targets[0]
for _, target := range targets {
t := make(map[string]interface{})
Copy link
Contributor

@Ninir Ninir Nov 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Nitpick: To be more memory-efficient, we could set: make(map[string]interface{}, 1), thoughts? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems the same

func hoge1() {
	results := make([]map[string]interface{}, 100)
	for i := range results {
		t := make(map[string]interface{})
		t["key"], t["values"] = "key", "value"
		results[i] = t
	}
}

func hoge2() {
	results := make([]map[string]interface{}, 100)
	for i := range results {
		t := make(map[string]interface{}, 1)
		t["key"], t["values"] = "key", "value"
		results[i] = t
	}
}

func hoge3() {
	results := make([]map[string]interface{}, 100)
	for i := range results {
		t := map[string]interface{}{}
		t["key"], t["values"] = "key", "value"
		results[i] = t
	}
}

func BenchmarkHoge1(b *testing.B) {
	for i := 0; i < b.N; i++ {
		hoge1()
	}
}

func BenchmarkHoge2(b *testing.B) {
	for i := 0; i < b.N; i++ {
		hoge2()
	}
}

func BenchmarkHoge3(b *testing.B) {
	for i := 0; i < b.N; i++ {
		hoge3()
	}
}

go test -bench BenchmarkHoge -benchmem
BenchmarkHoge1-4   	   50000	     32459 ns/op	   33600 B/op	     200 allocs/op
BenchmarkHoge2-4   	   50000	     32529 ns/op	   33600 B/op	     200 allocs/op
BenchmarkHoge3-4   	   50000	     36636 ns/op	   33600 B/op	     200 allocs/op

Copy link
Contributor

Choose a reason for hiding this comment

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

Third option is even better, interesting 🤔
Still, can you go for the second one, since it has a fixed size?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok👍

t["key"] = *target.Key
t["values"] = flattenStringList(target.Values)

t := make(map[string]interface{})
t["key"] = *target.Key
t["values"] = flattenStringList(target.Values)

result = append(result, t)
result = append(result, t)
}

return result
}
Expand Down
4 changes: 2 additions & 2 deletions website/docs/r/ssm_maintenance_window_target.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,11 @@ The following arguments are supported:

* `window_id` - (Required) The Id of the maintenance window to register the target with.
* `resource_type` - (Required) The type of target being registered with the Maintenance Window. Possible values `INSTANCE`.
* `targets` - (Required) The targets (either instances or tags). Instances are specified using Key=instanceids,Values=instanceid1,instanceid2. Tags are specified using Key=tag name,Values=tag value.
* `targets` - (Required) The targets (either instances or tags). Instances are specified using Key=instanceids,Values=instanceid1,instanceid2. Tags are specified using Key=tag name,Values=tag value. Minimum number of 0 items. Maximum number of 5 items.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove the first part of the change, since it is required? thanks

* `owner_information` - (Optional) User-provided value that will be included in any CloudWatch events raised while running tasks for these targets in this Maintenance Window.

## Attributes Reference

The following attributes are exported:

* `id` - The ID of the maintenance window target.
* `id` - The ID of the maintenance window target.