search

Oefenweb / ansible-postfix

Ansible role to set up postfix in Debian-like systems
MIT License
174 stars 82 forks source link

Quote no_log variable #99

Closed vladgh closed 4 years ago

vladgh commented 4 years ago

Addresses #98

vladgh commented 4 years ago

I am not sure about that error in TravisCI but I believe is a bug that appears in some version combinations of ansible-lint and Python. Looks similar to this: https://github.com/ansible/ansible-lint/issues/395

I am running some tests in docker with different Python versions this way:

docker run -it --rm -v /home/vlad/projects/forks/ansible-postfix:/ansible-postfix python:2.7 bash
$ pip install ansible-lint
$ ansible-lint /ansible-postfix/tests/test.yml

So it's up to you if you would consider upgrading Python to a newer version. Otherwise, I am not really sure how to troubleshoot this error.

vladgh commented 4 years ago

After numerous tests, I discovered what the problem is in the current TravisCI tests. According to this bug (https://github.com/ansible/ansible-lint/issues/892), ansible-lint 4.2.0 package is not compatible with ansible 2.10.x. It has been updated since then and the latest version (4.3.x) works. However, it needs at least Python 3.6 (python_requires = >=3.6). The newest version that can be installed in Python 2.7 or 3.5 used by these tests is 4.2.0 which is affected by this bug.

An elegant solution I can think of and that can keep backward compatibility would be to have the following 2 Python versions in the matrix. Keep Python 2.7 but pin ansible version to 2.9.13 (the last before 2.10) and replace Python 3.5 with 3.8.

I can open a separate PR for that if you wish, and rebase this one when that one succeeds.

dingouerinitx commented 4 years ago

Hello @tersmitten we use your role for our infrastructure and we would appreciate this fix as soon as possible. Your role worked for us a long time with ansible 2.7.10 but we are currently migrating to ansible 2.10 and this PR would help make it run again. Kind regards

tersmitten commented 4 years ago

I'd love to merge this, but I want to be sure that this works on older versions