ansible-postgresql_exec custom module error ValueError: I/O operation on closed file

[gilesw]

ubuntu, running off pips.

ansible 2.7.8
  config file = /home.local/gilesadm/work/ansible-config/ansible.cfg
  configured module search path = [u'/home.local/gilesadm/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /opt/python-tools/local/lib/python2.7/site-packages/ansible
  executable location = /opt/python-tools/bin/ansible
  python version = 2.7.3 (default, Oct 26 2016, 21:01:49) [GCC 4.6.3]

I'm hitting an error running a custom module I use:-

gilesw/ansible-postgresql_exec#1

Hopefully a simple one to fix.

[dw]

This one looks interesting -- I bet it's something to do with a difference in stdin/stdout. Should have time to look at this over the weekend, thanks for reporting!

[dw]

It's almost certainly due to

# fixes https://github.com/ansible/ansible/issues/3518                                 
import sys                                                                             
reload(sys)                                                                            
sys.setdefaultencoding('utf8')                                                         
import pipes            

Due to ansible/ansible#3518

[dw]

GitHub shows this is an unfortunately common idiom in Python: https://github.com/search?p=3&q=%22sys.setdefaultencoding%28%27utf8%27%29+%22&type=Code

But much less so in Ansible: https://github.com/search?q=%22sys.setdefaultencoding%28%27utf8%27%29++import+pipes%22&type=Code

[dw]

It's been gone from the Ansible tree completely since Aug 2015, so this is just about third party compatibility

$ git log -S "sys.setdefaultencoding"  HEAD

commit ef594f708c6eca9ad45d4926b942cf6ce0f15ee1
Author: Brian Coca <brian.coca+git@gmail.com>
Date:   Thu Aug 27 12:27:38 2015 -0400

    remove old dead code

commit 43c1a9744765eebfb9eaf9113336d552cfc9096b
Author: Toshio Kuratomi <toshio@fedoraproject.org>
Date:   Mon Mar 30 19:19:34 2015 -0700

    Various unicode and backslash escape cleanups
    
    * Do backslash escape parsing in parse_kv() [was being done in the copy
      module purely for newlines in the copy module's content param before]
    * Make parse_kv always return unicode
    * Add bandaid to transform args to unicode until we can fix things
      calling parse_kv to always send it unicode.
    * Make split_args deal with unicode internally.  Warning, no bandaid for
      things calling split_args without giving it unicode (shouldn't matter
      as dealt with str internally before)
    * Fix copy and unarchive action plugins to not use setdefaultencoding
    * Remove escaping from copy (it was broken and made content into latin-1
      sometimes). escaping is now in parse_kv.
    * Expect that content is now a unicode string so transform to bytes just
      before writing to the file.
    * Add initial unittests for split_args and parse_kv.  4 failing
      tests.because split_args is injecting extra newlines.

commit 065733ad93abcc0031bbd3578bfc34b4f31a1753
Author: James Cammarata <jimi@sngx.net>
Date:   Fri Jan 2 07:51:15 2015 -0600

    Moving more action plugins over and fixing some bugs with role loading

commit 62d79568be16084718bda2d890b2b4e1d10cc41d
Author: James Cammarata <jimi@sngx.net>
Date:   Fri Nov 14 16:14:08 2014 -0600

    Creating playbook executor and dependent classes

commit bf916fb58a351ee409ef5bbb3899079712226ab7
Author: root <root@TINY.(none)>
Date:   Mon Nov 24 18:03:32 2014 +0000

    Adding first pass at win_copy, win_file and win_template modules.

commit 5062f4962f8e7939c4656a059fc86e0fb4a3bf55
Author: Matt Martz <matt@sivel.net>
Date:   Fri Mar 7 21:51:12 2014 -0600

    Unit tests for ansible.utils

commit c0a7f516714bc394c06eacd5be5a12e86411248e
Author: Matthew Thode <mthode@mthode.org>
Date:   Mon Dec 9 12:49:03 2013 -0600

    tests depend on the default encoding being utf8
    
    So we set the utils default encoding to be utf8

commit 2c28e1daea2fee5edccd6860bf493a4d8888b4a7
Author: Dylan Martin <dmartin@sccd.ctc.edu>
Date:   Mon Oct 21 15:22:42 2013 -0500

    unarchive module & action_plugin added

commit 9991a530ab039a2407344a295177ccfeb0f3a2b6
Author: Serge van Ginderachter <serge@vanginderachter.be>
Date:   Thu Aug 8 22:09:45 2013 +0200

    fix an encoding bug in copy content = lookup plugin
    
    closes #3518

---

This might warrant a monkey-patch. The alternative is breaking compatibility with ancient existing modules. The question is what the patch should do!

Some obvious behaviours

• reload() on sys should silently succeed with no effect, that's a crazy hack to begin with
• setdefaultencoding can be used to reopen our redirected stdio as UTF-8, without tearing apart the whole sys module

The alternative is to stop relying on sys.stdout redirects, and instead rely on FD redirection. It's slower but a lot more robust.

[dw]

Hi @gilesw

I've been over this module a few times, and I'm a little perplexed what the setdefaultencoding() workaround is being used for. It's naturally been made obsolete in newer Ansible versions, but that doesn't explain why it is needed in older Ansibles.

For example on Python 2, the 'read_file()' function will yield bytes, while on Python 3 it will decode UTF-8 by default

The only remaining use is on line 279, where "getdefaultencoding()" is used instead of simply 'utf-8' or similar.

Mitogen takes compatibility very seriously, but in this case, it'd be nice to understand what we're trying to be compatible with first before fixing it :)

[gilesw]

@dw thanks for looking into this. I'm afraid I'm not the original author, or any kind of python programmer. I just looked at the postgresql modules that are part of Ansible as a sort of template to make recent modifications and added output from the query that's run (which seems vital to me.)

[dw]

This is now on the master branch and will make it into the next release. To be updated when a new release is made, subscribe to https://networkgenomics.com/mail/mitogen-announce/

Thanks for reporting this!