Skip to content

Don't request region unless we are on ec2#115

Closed
ubiquitousthey wants to merge 3 commits intosous-chefs:masterfrom
ubiquitousthey:master
Closed

Don't request region unless we are on ec2#115
ubiquitousthey wants to merge 3 commits intosous-chefs:masterfrom
ubiquitousthey:master

Conversation

@ubiquitousthey
Copy link
Contributor

If we aren't on ec2 (and we are probably using s3) just default to us-east-1

@scalp42
Copy link
Contributor

scalp42 commented Mar 4, 2015

Looks good 👍

@asivitz
Copy link

asivitz commented Mar 9, 2015

Could we specify the region as an attribute? Seems like a better longterm solution.

@chef-supermarket
Copy link

Hi. I am an automated pull request bot named Curry. There are commits in this pull request whose authors are not yet authorized to contribute to Chef Software, Inc. projects or are using a non-GitHub verified email address. To become authorized to contribute, you will need to sign the Contributor License Agreement (CLA) as an individual or on behalf of your company. You can read more on Chef's blog.

GitHub Users Who Are Not Authorized To Contribute

The following GitHub users do not appear to have signed a CLA:

Please sign the CLA here.

@asivitz
Copy link

asivitz commented Mar 10, 2015

Oh jeez. I don't have a supermarket account. You can revert my commit, or make a similar commit on a different branch. Or call it an Obvious Fix or whatever.

@chef-supermarket
Copy link

Hi. Your friendly Curry bot here. Just letting you know that all commit authors have become authorized to contribute. I have added the "Signed CLA" label to this issue so it can easily be found in the future.

@ubiquitousthey
Copy link
Contributor Author

OK. I have fixed this and it pulls from an attribute. Please merge.

@miketheman
Copy link
Contributor

@ubiquitousthey I think this looks legit.

One note I'd make is that there's a pattern in this library for creating methods that encapsulate the logic behind querying for a variable and responding with 'something' - see examples of instance_availability_zone - which uses this kind of pattern.

You might want to factor that out to a method, then squash commits to give this another go.

@mkantor
Copy link
Contributor

mkantor commented Jun 15, 2015

@miketheman do you mean something like this?

Looking up the region is a bit different from the other query_*-based stuff because it's not a separate query (the region is parsed out of the availability zone).

@miketheman
Copy link
Contributor

@mkantor I think I see your point - as well as adding the extra method may fail itself.

I guess if this is squashed, we can proceed.

@scalp42
Copy link
Contributor

scalp42 commented Jun 16, 2015

@ubiquitousthey any chance you can make it happen please? 🙏

@ubiquitousthey
Copy link
Contributor Author

OK. See PR #160 for the squashed commits

@ubiquitousthey
Copy link
Contributor Author

Closing this in favor of #160.

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.

6 participants