Fix travis build by fixing ansible-lint version #487

Merged
merged 1 commit into from Mar 5, 2016

Projects

None yet

3 participants

@spk
Contributor
spk commented Feb 11, 2016

cf #486

@ariddell
Contributor

I'll merge this but what exactly does it fix?

@carljm
Contributor
carljm commented Feb 11, 2016

Should fix the Ansible lint errors on #486, which are unrelated to that PR.

@spk
Contributor
spk commented Feb 11, 2016

Yes we have to use last ansible version for syntax check and a fixed version for ansible-lint.
I don't have better solution right now.

@ariddell
Contributor

Sorry, I'm confused. Why can't the requirements.txt file be used? Shouldn't the travis.yml file just have a line pip install -r requirements.txt?

@spk
Contributor
spk commented Feb 11, 2016

No I tried in this commit spk@3daf7cf
but ansible-lint failed completely due to too old ansible version.
cf https://travis-ci.org/sovereign/sovereign/builds/108639408#L443

@ariddell
Contributor

So the problem is with ansible. Why are we pinning to 1.6.6 anyway?

Can you go back to spk@3daf7cf and figure out a version of ansible that works?

If you're feeling up to it, the ansible-lint requirement probably should go in test-requirements.txt. I believe that's the convention.

@spk
Contributor
spk commented Feb 12, 2016

I cannot upgrade ansible, this is a known issue, cf #466
I did not know about test-requirements.txt, I will do that.

@ariddell ariddell referenced this pull request Feb 12, 2016
Closed

Become #466

@ariddell ariddell and 1 other commented on an outdated diff Feb 15, 2016
install:
- - pip install ansible ansible-lint
+ - pip install ansible
@ariddell
ariddell Feb 15, 2016 Contributor

this should be pip install -r requirements.txt (to get ansible)

@spk
spk Feb 16, 2016 Contributor

Should I rebase on jessie branch ? because on master ansible version 1.6 is too old ?

@ariddell ariddell commented on the diff Feb 15, 2016
requirements.txt
@@ -1,2 +1 @@
ansible==1.6.6
@ariddell
ariddell Feb 15, 2016 Contributor

What happens if this is something like
ansible>=1.9.4,<2.0 instead of a fixed version?

@ariddell ariddell commented on the diff Feb 15, 2016
test-requirements.txt
@@ -0,0 +1 @@
+ansible-lint==2.3.6
@ariddell
ariddell Feb 15, 2016 Contributor

Is this the only version of ansible-lint that works or can we use a version range like
ansible-lint>=2.3.6,<3.0

@spk
spk Feb 16, 2016 Contributor

Only ansible-lint 2.3.6 is working after there will be sudo/become errors that are not merged yet.

@ariddell
ariddell Feb 16, 2016 Contributor

This PR is confusing to me. What problem is being solved here? A more
descriptive PR text would be helpful. Travis is currently building
successfully on master.

On 02/16, Laurent Arnoud wrote:

@@ -0,0 +1 @@
+ansible-lint==2.3.6

Only ansible-lint 2.3.6 is working after there will be sudo/become errors that are not merged yet.


Reply to this email directly or view it on GitHub:
https://github.com/sovereign/sovereign/pull/487/files#r53078021

@spk spk changed the title from Use ansible from requirements for travis to Fix travis build by fixing ansible-lint Feb 17, 2016
@spk spk changed the title from Fix travis build by fixing ansible-lint to Fix travis build by fixing ansible-lint version Feb 17, 2016
@spk
Contributor
spk commented Mar 5, 2016

Title updated. Currently ansible is fixed to 1.6.6, this version does not work with ansible-lint.
And there is still sudo on the codebase that cause errors so fixed ansible-lint to 2.3.6

@ariddell
Contributor
ariddell commented Mar 5, 2016

OK. I think I get this now. Sorry for being so slow.

Can you squash/rebase all those commits into a single commit (since it's a simple change)?

p.s. maybe then we can resolve #446 by upgrading both ansible and ansible-lint to the current version (maybe just on jessie)?

@spk spk Use ansible-lint==2.3.6
Before sudo rule merge
willthames/ansible-lint#96

* Add test-requirements.txt
* travis: enable cache for pip packages
bfe6fe8
@spk
Contributor
spk commented Mar 5, 2016

No problem @ariddell
This is done and I also added a todo on .travis.yml

@ariddell ariddell merged commit c70a2fa into sovereign:master Mar 5, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ariddell ariddell referenced this pull request Mar 7, 2016
Merged

Update ansible in jessie #508

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