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

autoscaler attach/detatch #229

Merged
merged 4 commits into from
Mar 4, 2016

Conversation

shaiguitar
Copy link

Ran this against a live amazon account and this did the expected.

Amazon doc links inline in the commit.

@shaiguitar
Copy link
Author

/cc @lanej

#
# ==== See Also
#
# http://docs.aws.amazon.com/cli/latest/reference/autoscaling/attach-instances.html
Copy link
Member

Choose a reason for hiding this comment

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

@shaiguitar
Copy link
Author

pushed doc link change

@@ -1,5 +1,5 @@
module Fog
module AWS
VERSION = "0.8.1"
VERSION = "0.9.0"
Copy link
Member

Choose a reason for hiding this comment

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

i'll worry about the version bump, tagging, etc.

@shaiguitar
Copy link
Author

reverted lib/version.rb change to what it was

@geemus
Copy link
Member

geemus commented Feb 25, 2016

@lanej looks like you are on top of this, but just let me know if you need anything.

@lanej
Copy link
Member

lanej commented Feb 25, 2016

@geemus thanks

@lanej
Copy link
Member

lanej commented Feb 25, 2016

@shaiguitar the mocks don't manipulate any data. is that intentional ?

@shaiguitar
Copy link
Author

@lanej Basically just copied over what was in https://github.com/fog/fog-aws/blob/master/lib/fog/aws/requests/auto_scaling/attach_load_balancers.rb#L36-L55 . There's no documentation/wiki on adding mock behavior and it's been a while so I'm a little hazy on the idiomatics.

Seems like the asg mock interactions are broken as well with regards to the other instance manipulation requests, see these two (Fog::Mock.not_implemented) lib/fog/aws/requests/auto_scaling/set_instance_health.rb and lib/fog/aws/requests/auto_scaling/terminate_instance_in_auto_scaling_group.rb

In the case of lib/fog/aws/requests/auto_scaling/set_desired_capacity.rb if we reload after setting desired capacity it should also create some instances, but nothing is happening in the mock either.

There's probably some other examples but that should be an indication it's not quite there for the asgs anyway.

So, I can add another not_implemented in there or I can take a look at implementing these mocks, but I'll need some hand holding and or docs/wiki.

@lanej lanej self-assigned this Feb 25, 2016
@shaiguitar
Copy link
Author

Bump. The real implementation should work. I can add in not_implemented if that's better than the current alternative

@shaiguitar
Copy link
Author

Anything I can do to get this released?

@lanej
Copy link
Member

lanej commented Mar 4, 2016

@shaiguitar rebase for a passing build

Shai Rosenfeld added 4 commits March 4, 2016 10:12
Amazon doc links inline in the commits
@shaiguitar shaiguitar force-pushed the autoscaler_instance_attach_detach branch from 6d46888 to 2456533 Compare March 4, 2016 18:12
@shaiguitar
Copy link
Author

@lanej green

lanej added a commit that referenced this pull request Mar 4, 2016
@lanej lanej merged commit 6793cd5 into fog:master Mar 4, 2016
@shaiguitar shaiguitar deleted the autoscaler_instance_attach_detach branch March 4, 2016 19:10
@shaiguitar
Copy link
Author

Thanks!

@lanej
Copy link
Member

lanej commented Mar 4, 2016

@shaiguitar this commit screwed with the versions @2456533f029698f060f4c368af22d9106d74b64f. Next time remove the commit instead of reverting in the same branch.

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