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 an attribute to define region when not in ec2 #160

Merged
merged 1 commit into from Nov 25, 2015

Conversation

Projects
None yet
9 participants
@ubiquitousthey
Contributor

ubiquitousthey commented Jun 30, 2015

No description provided.

@martinisoft

This comment has been minimized.

Show comment
Hide comment
@martinisoft

martinisoft Jun 30, 2015

Did you want to keep this separate from #115 or close that one in favor of this PR? Would love to see this get merged so I can unpin my much older cookbook. 👍

martinisoft commented Jun 30, 2015

Did you want to keep this separate from #115 or close that one in favor of this PR? Would love to see this get merged so I can unpin my much older cookbook. 👍

@ubiquitousthey

This comment has been minimized.

Show comment
Hide comment
@ubiquitousthey

ubiquitousthey Jun 30, 2015

Contributor

This is a squashed version of #115. I'll close it.

Contributor

ubiquitousthey commented Jun 30, 2015

This is a squashed version of #115. I'll close it.

@ubiquitousthey

This comment has been minimized.

Show comment
Hide comment
@ubiquitousthey

ubiquitousthey Jun 30, 2015

Contributor

I, and others, would love to see this in milestone:2.7.2.

Contributor

ubiquitousthey commented Jun 30, 2015

I, and others, would love to see this in milestone:2.7.2.

@miketheman miketheman added this to the 2.8.0 milestone Jun 30, 2015

@miketheman

This comment has been minimized.

Show comment
Hide comment
@miketheman

miketheman Jun 30, 2015

Collaborator

Thanks! I've tagged for the 2.8.0 release, which will include enhancements and features. 2.7.2 has been released already.

Side note: I think we definitely need to take a stab at adding spec testing to this cookbook, as this has lagged behind due to the remote API complexity, but the newer SDK has some nice stubbing/mocking abilities that should prove worthwhile.

Collaborator

miketheman commented Jun 30, 2015

Thanks! I've tagged for the 2.8.0 release, which will include enhancements and features. 2.7.2 has been released already.

Side note: I think we definitely need to take a stab at adding spec testing to this cookbook, as this has lagged behind due to the remote API complexity, but the newer SDK has some nice stubbing/mocking abilities that should prove worthwhile.

@joshzitting

This comment has been minimized.

Show comment
Hide comment
@joshzitting

joshzitting Jul 10, 2015

I have a question on this...
If we are deploying instances say in us-west-1 but my s3 bucket is in us-west-2 will it screw up all of the outher uses of this cookbook by having this attribute set??? I would sure prefer just to have a variable that I set in the aws_s3_file LWRP for that particular file???

Or will this not be an issue??

joshzitting commented Jul 10, 2015

I have a question on this...
If we are deploying instances say in us-west-1 but my s3 bucket is in us-west-2 will it screw up all of the outher uses of this cookbook by having this attribute set??? I would sure prefer just to have a variable that I set in the aws_s3_file LWRP for that particular file???

Or will this not be an issue??

@martinisoft

This comment has been minimized.

Show comment
Hide comment
@martinisoft

martinisoft Jul 10, 2015

@joshzitting Good point, that solution is being proposed in #149 so I am wondering if we need to merge these solutions together.

Have an attribute default that the LWRP defaults to, but allow for it to be changed with the LWRP defined resource. Otherwise I am in favor of merging #149 first then changing this solution to add that.

martinisoft commented Jul 10, 2015

@joshzitting Good point, that solution is being proposed in #149 so I am wondering if we need to merge these solutions together.

Have an attribute default that the LWRP defaults to, but allow for it to be changed with the LWRP defined resource. Otherwise I am in favor of merging #149 first then changing this solution to add that.

@SIGUSR2

This comment has been minimized.

Show comment
Hide comment
@SIGUSR2

SIGUSR2 Jul 10, 2015

+1 @joshzitting @martinisoft ... fits exactly the way I use this resource

SIGUSR2 commented Jul 10, 2015

+1 @joshzitting @martinisoft ... fits exactly the way I use this resource

@rojomisin

This comment has been minimized.

Show comment
Hide comment
@rojomisin

rojomisin Aug 19, 2015

do we know which version of the cookbook introduced this? 2.5.0 seems to be ok, 2.7.2 has this bug.

rojomisin commented Aug 19, 2015

do we know which version of the cookbook introduced this? 2.5.0 seems to be ok, 2.7.2 has this bug.

@martinisoft

This comment has been minimized.

Show comment
Hide comment
@martinisoft

martinisoft Aug 19, 2015

@rojomisin Version 2.6.0 is where they changed the gem which caused the break.

martinisoft commented Aug 19, 2015

@rojomisin Version 2.6.0 is where they changed the gem which caused the break.

@davidski

This comment has been minimized.

Show comment
Hide comment
@davidski

davidski Oct 31, 2015

As a new user kicking the tires on this cookbook on a local vagrant instance before moving to AWS, hitting the dependency on the instance metadata under the 2.7.x branches is a rude bump. Would dearly love to see this merged and a release cut to supermarket. Thanks!

davidski commented Oct 31, 2015

As a new user kicking the tires on this cookbook on a local vagrant instance before moving to AWS, hitting the dependency on the instance metadata under the 2.7.x branches is a rude bump. Would dearly love to see this merged and a release cut to supermarket. Thanks!

@miketheman

This comment has been minimized.

Show comment
Hide comment
@miketheman

miketheman Nov 25, 2015

Collaborator

@tas50 I'm a 👍 on this.

Collaborator

miketheman commented Nov 25, 2015

@tas50 I'm a 👍 on this.

tas50 added a commit that referenced this pull request Nov 25, 2015

@tas50 tas50 merged commit 7c68788 into chef-cookbooks:master Nov 25, 2015

@tas50 tas50 changed the title from Use attribute for region if not in ec2 to Add an attribute to define region when not in ec2 Jan 21, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment