Fix for accounts without EC2-Classic #52
Conversation
…that doesn't support EC2-Classic
|
@philchristensen thanks so much for this. The failed Travis tests are a known issue, #46, that I need to deal with. The code looks simple enough. I should be able to get this merged and released in the next week. Apologies for the delays, but I'm spread pretty thin lately, and the integration test issue is going to take some work. |
|
No problem, I was thrilled to find this . I also have a big PR on the way to support using sts:AssumeRole to do cross-account lookups. I'm using it now to scan service limits for all my agency's client accounts, and will submit it once I'm sure it's working right. |
|
|
|
For this stuff, would you mind please:
If that's too much to ask, I can probably get around to it sometime this week. As to your big PR that's coming, I'm going to need to think on that a bit; I hadn't really planned on adding multi-account support to awslimitchecker itself in favor of just setting the correct environment variables for boto for each account in question, though I know that doesn't really solve the same problem as STS. Also, please note how this handles regions - I don't know how STS interacts with regions (I've never used it myself), but that could be a potential issue. I've never used STS before, and I'm not sure I have an easy way to test it, so please note that in order to merge, I'd really like it to be fully documented and have complete test coverage. |
|
As for this patch, no prob, glad to update it. The other stuff may not be of particular use to you, but if I do make a PR I'll be sure to follow your testing standards. |
|
I'll be willing to make the fixes to this patch if you want - I believe I can just pull this in to my own branch and add the tests. I'd like to cut a release with this and a fix for #54 sometime today. As to the sts:AssumeRole, I actually realized this will make some things easier for me as well, so please submit the PR when you have it, and I'll help (or handle) any major issues with tests/etc. |
|
If you want to take care of this one, go for it. I'll submit the STS with better tests, but if you'd like to take a look at Logicworks/awslimitchecker@34444019c63b43d14eec79df4afd70a37e993b7e The biggest change is to the service classes' connect() functions. I didn't want to have to force people to supply a region, but that's required for STS. So I've made it so it uses the original connection method if region=None (e.g., defined in the OS), otherwise it checks if account=None, and if not, uses STS. Otherwise it's a normal connection to the provided region. |
|
Ok, I'll handle this EC2-Classic stuff myself, hopefully tonight. I'll have to close this PR and open another, but I'll keep your original commits to make sure you show up in the contributors graph. I glanced over the STS stuff, and not only does it look really good, but pretty complete too (you even updated the newservice template - thanks!) |
|
I'm closing this in favor of #58 that has some unit tests added and some style fixes. Please open a PR for the STS stuff when you have it done... I'm really interested in it. |
|
+1. Will try to get that other branch in this weekend. |
|
Ok, cool. PyPi says this has 681 downloads in the ~2 weeks since I released it, which is far more than I'd ever expected. I know very well the frustration of OSS projects not being responsive to pull requests, so I'm trying to do my best. If it seems otherwise, feel free to call me out on it. I'm all for incorporating features that others need regardless of whether I need them or not. |
|
I hear that too. I maintain a OSS project called django-salesforce and I have the same philosophy. I'm pretty sure I'll be able to get that in this weekend. -Phil
|
|
This has been released in 0.1.1, which is now live on pypi. |
New AWS accounts these days don't support EC2-Classic, and strangely raise
BotoServerErrors when you query for ElasticCache cache security groups (in EC2-VPC you just use regular VPC security groups).I can't find any way to check the available platforms from boto, so I'm just catching the server error in this PR.