jdauphant / ansible-role-nginx

Ansible role to install and manage nginx configuration
655 stars 302 forks source link

Errors with configuration.yml and Python 3 #150

Closed tyxieblub closed 7 years ago

tyxieblub commented 7 years ago

Hello,

Using Python 3.5.2 and Ansible 2.2.0, I have encountered an error while generating a config with nginx_auth_basic_files and nginx_configs, empty or not.

Whether those are empty or not, I get the following error (with an empty list, but the dict_keys can be populated and it is the same):

failed: [nginx] (item=dict_keys([])) => {"failed": true, "item": "dict_keys([])", "msg": "AnsibleUndefinedVariable: 'dict object' has no attribute \"dict_keys([])\""}

This seems to work for nginx_sites thanks to the | difference(nginx_remove_sites) so I managed to make it work the same way. However, this did not resolve the problem for the case when the dict is empty since it returns an empty set, so I added a workaround using when for this case. Here is what I got for nginx_auth_basic_files:

- name: Ensure auth_basic files created
  template:
    src: auth_basic.j2
    dest: "{{nginx_conf_dir}}/auth_basic/{{ item }}"
    owner: root
    group: "{{nginx_group}}"
    mode: 0750
    with_items: "{{ nginx_auth_basic_files.keys() |  difference([]) }}"
  when:  nginx_auth_basic_files != {}

Would this be the right way of fixing this problem?

jdauphant commented 7 years ago

Hello @tyxieblub , Sorry for the late answer, I just see your message.

If I understand well, Ansible Python 3 support was introduced in the last version 2.2 and is in tech review.

It's maybe better to replace all with_items: {{dict.keys()}} by with_dict: dict : http://docs.ansible.com/ansible/playbooks_loops.html#looping-over-hashes (we may have to adapt the template too, like nginx_auth_basic_files[item] will become item.value) It should be more simple and compatible with Python 3.

What do you think ? Julien

tyxieblub commented 7 years ago

Hello @jdauphant, No worries it seems I am a bit late too…

Indeed, I avoided changing the templates and to focus on the tasks, since some dicts were working fine. I just tried it with with_dict and it works wonderfully, whether the dict is empty or not.

Hash loops were introduced in Ansible 1.5 so there should be no impact on the requirements of the project.

If you want I can adapt the tasks/templates and do a PR. However, I'm not sure with_dict would be more straightforward when dealing with the difference filter (like the symlink task) so if I do so, I may not touch them.

jdauphant commented 7 years ago

@tyxieblub A PR will be welcome :)

For symlink, something like that should work:

- name: Create links for sites-enabled
  file:
    state: link
    src: "{{nginx_conf_dir}}/sites-available/{{ item.key }}.conf"
    dest: "{{nginx_conf_dir}}/sites-enabled/{{ item.key }}.conf"
  with_dict: "{{ nginx_sites }}"
  when:  item.key not in nginx_remove_sites
  notify:
   - reload nginx
tyxieblub commented 7 years ago

@jdauphant Done in #153 !

It seems like the build passed without problems. However it may pose a problem for people with custom templates using these variables as they cannot access them with nginx_sites[item].var anymore and should now use item.value.var.

jdauphant commented 7 years ago

Yep good point, we should give a notice about that in the Release Details