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

update requirements.txt file to use latest boto3. #59

Closed

Conversation

Susanthab
Copy link

Changed boto3 version of the requirements.txt file to use latest versions.
e.g: boto3>=1.4.5 ( was, boto3**=**=1.4.5)

@Susanthab
Copy link
Author

Could you please review this PR and accept or deny? This change is required to use new boto3 features via Stackstrom.

@brainstorm
Copy link

@Susanthab There are clearly errors on your changes that break the build:

https://circleci.com/gh/StackStorm-Exchange/stackstorm-aws/149

I guess that fixing those first would give way to a review and eventual merge of your PR?

@brainstorm
Copy link

Also, see issue #55, this is being actively discussed since August since there are breaking changes between boto2/3 and the pack's different components.

@brainstorm
Copy link

Also, this PR is trying to tackle the problem in another way?: StackStorm-Exchange/exchange-incubator#10

@LindsayHill
Copy link
Contributor

I haven't looked closely at the CircleCI failures, but it's possible that it is failing because CircleCI is running checks against master, not against the boto3 branch that this PR wants to update.

Could be that we need to update our CircleCI config to handle running tests against a different branch.

Also, on the PR itself, I'm not a big fan of specifying a minimum version requirement with no maximum. I'd rather have something like boto3>=1.4.5,<1.5. That way we're less likely to have breakage when upstream libraries change behavior when they move to 1.5.x.

I also don't understand how this PR is needed to use new boto3 features - the current pack installs 1.4.5, and this change is to go to >=1.4.5 - so if I already have 1.4.5 installed, it won't change anything. If you need something in 1.4.6 or higher, it might be better to specify that?

@Susanthab
Copy link
Author

Susanthab commented Dec 12, 2017

I found two issues mentioned below with the current boto3 st2 pack;
boto/boto3#1349
boto/boto3#1353
So is there an alternative way to address these two without changing the boto3 version in requirements file?

@LindsayHill
Copy link
Contributor

You may well need to change the version. But just changing it to >=1.4.5 won't work. If you need 1.4.7, then you should specify that with something like ==1.4.7, or >=1.4.7,<1.5.

We also need to figure out why CircleCI tests are failing. It may be that some other actions need updating to reflect changes in the library

@AndyMoore
Copy link
Contributor

I have 109 new aws packs ready to go. I've been using about 10 of them locally in testing and all seem good (they're derivatives of the current pack anyway). They use latest boto3, and so should solve your problems. @Susanthab if you want to message me i can show you where they are - it'd be handy to have a second set of eyes on them before committing them to the incubator

@AndyMoore
Copy link
Contributor

also the new stackstorm-aws_ec2 pack has the following action:

-rw-rw-r-- 1 andy andy 1268 Nov 13 23:29 update_security_group_rule_descriptions_ingress.yaml

@brainstorm
Copy link

@AndyMoore is it safe to st2 pack install your AWS packs from your repo(s) or would you be introducing some more changes and then pushing upstream soon and deleting those from your account?

@AndyMoore
Copy link
Contributor

@brainstorm They already have fixes after a review from Warren so I don't expect them to change much from now - I was just trying to give them some proper testing before resubmitting to incubator

Susanthab and others added 2 commits December 13, 2017 10:05
If you put `>=1.4.5`, it may only install 1.4.5, but you need a minimum of 1.4.7 to get the features you want.
@Susanthab
Copy link
Author

I incorporated the review comments to specify the max version limit for boto3. However the CircileCi test is failing and I'm not sure why. I believe this test is running against the master branch where as this change is on boto3 branch.

@Susanthab
Copy link
Author

This fix is working for me. I'm able to use new boto3 features with this requirement.txt.

@LindsayHill
Copy link
Contributor

@Susanthab I modified your PR to require >= 1.4.7, since that's the version you need (>=1.4.5 does not guarantee that you will get 1.4.7, it would be satisfied by 1.4.5 or 1.4.6).

I'm pretty sure the CircleCI failures are indeed because it's testing against the wrong branch, but I haven't had a chance to investigate that yet.

@Susanthab
Copy link
Author

@LindsayHill Thank you.

@LindsayHill
Copy link
Contributor

Looking a bit closer at CircleCI, I think the problems are related to it trying to test a forked branch against this one. It should work if I do it from another branch in this repo - I've submitted #60 to replace this one

@Susanthab
Copy link
Author

@LindsayHill Thanks.

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.

4 participants