Aws autoscaling/fix tag assignment

[restebanez]

hi @geemus,

This PR fixes the tags assignment when an auto scaling group is created. It now accepts a hash instead of an array of hashes.

Rodrigo

[coveralls]

Coverage remained the same when pulling 4576b38 on restebanez:aws_autoscaling/fix-group-creation into 31f74ce on fog:master.

[harryw]

it looks like you could still use each_with_index here, just without the inner each loop.

[geemus]

I think each_with_index would be a bit cleaner, just need to ensure you do index+1 since it starts at 1.

[restebanez]

Good idea!

[geemus]

@restebanez I'm not sure if I follow the issue related to defaults. Could you explain an example maybe? Thanks!

[coveralls]

Coverage remained the same when pulling 4bf665b on restebanez:aws_autoscaling/fix-group-creation into 31f74ce on fog:master.

[restebanez]

I noticed the error in a one month old branch, i didn't realize that it was solved 12 days ago: #2161. Sorry for the confusion.

The tags problem wasn't fixed so i'll keep the PR with that part

[coveralls]

Coverage remained the same when pulling 557204e on restebanez:aws_autoscaling/fix-group-creation into 31f74ce on fog:master.

[geemus]

@restebanez - no problem, just wanted to focus on the problem-at-hand if possible. Thanks!

[tlunter]

So since this now takes just a hash, the other possible parameters can't be sent to AWS. Also, the documentation wasn't fixed above and is now wrong.

This seems more detrimental than good.

[tlunter]

Also, it seems that the mock in this case is also quite wrong. The result from AWS would be what was originally being passed in: An array of hashes. What's being passed in is now just a hash. There should be logic around this.

[geemus]

@tlunter not sure I follow, at a glance at least it appears that the non-tag parameters should go through as before. Could you elaborate on what I missed so I can work to fix it? Thanks.

[tlunter]

@geemus
Previously the data structure for tags looked like:

[
  {
    'Key' => 'my_key',
    'Value' => 'my_value',
    'PropagateAtLaunch' => 'false',
    'ResourceId' => '',
    'ResourceType' => ''
  }
]

Now all of these weren't necessary every time, but should anyone wish to change values they are now locked out of it. Also, the response from describe_auto_scaling_groups comes back like this, not as a hash like

{
   'Key' => 'Value'
}

The mock in this case is incorrect since the mapping of tags changes on the response. The Real response has tags mapped like the top version, but the Mock response has tags like the bottom.

My comment on the documentation being wrong is directly tied to the Tags portion.

[geemus]

@tlunter got it. Would you be up for helping us fix it?

[tlunter]

I'll be taking a look into it.

[geemus]

Awesome, thank you.

On Thu, Nov 14, 2013 at 8:39 AM, Todd Lunter notifications@github.comwrote:

I'll be taking a look into it.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/2208#issuecomment-28488409
.