Fix http_request for IPv6-only machines

[ei-grad]

ansible.module_utils.urls creates socket with AF_INET family, which
doesn't work for IPv6-only machine.

[bcoca]

the issue you point at in the title seems to have been closed a long time

[ei-grad]

If you look down throught the comments you will find that there are another (unreported) issues messing with that one. Also, see #17645.

[ei-grad]

But you are right, I should probably remove its mention from the commit message.

[bcoca]

this breaks 2.4 compatibility as it was added in 2.6

[ei-grad]

@bcoca is there a common practice to backport such things?
Should I just copy it to ansible.module_utils.urls?
https://github.com/python/cpython/blob/master/Lib/socket.py#L688

[abadger]

@jimi-c made the following suggestion:

Since we're going to be stopping support of python-2.4 in new versions of Ansible in April of next year, perhaps it makes sense to use create_connection() if it's available, otherwise using socket.connect(). Would also need a note that the code won't work with ipv6 on python-2.4.

[ei-grad]

Fixed in 4e19451878779023bba0679baa35baba1d9e96d7

[sivel]

I'm not really a fan of this approach, and generally speaking, we have tried to stay away from it.

I am more in favor of just doing:

try:
    sock = socket.create_connection(address)
except AttributeError:
    sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
    sock.connect(address)

[ei-grad]

And I'm definitely is not fan of abusing try/except for non-exceptional situations. Your code would raise and handle exception every time on python 2.4. This is not something that could be called "python 2.4" support...

[abadger]

I don't have a huge preference here but I think @sivel's code is slightly better. We do use try: except (Ask forgiveness) quite a lot for compatibility between python versions. Asking forgiveness seems to be the accepted idiomatice way to write python versus look before you leap.

Since this is for python-2.4 only it makes even more sense. In the large picture there's only a few users running python-2.4 and we're going to be getting rid of support for it in less than a year.

Probably the major problem with the approach currently in the code is that it creates an ambiguity in what happens when create_connection() is called that isn't apparent when you are looking at the caller. This can be worked around by adding the new function under a different name and then putting the conditional into the calling code to select which of the two functions to call or putting the condition and the implementing code into the caller (The latter repeats itself once but for this specific case is probably okay -- it adds less lines of code which often indicates that the function can be considered too trivial to warrant being moved outside the caller). That will be slower for the common case of operating on non-python-2.4 (exceptions are faster than conditionals when following the happy path) but may be a good compromise. @sivel is the exception vs conditional the major thing you're looking at or the obfuscation of create_connection's meaning and the relative burden between caller and called?

[sivel]

Sorry for the slow response, I am concerned both with pythonic implementations, speed and obfuscation combined.

Creating a function named the same as the original method, by which it is hard to tell in the calling location which is happening will often lead to confusion.

hasattr can also sometimes work in strange ways such as what is described in https://hynek.me/articles/hasattr/

Additionally, with hasattr you are performing the lookup multiple times, instead of just trying to use it and handle the exception. In the majority of cases, just trying to use it and handling the exception will be the faster way.

[ei-grad]

I don't create a function named the same as original method, I just use it if it is available, and define a fallback if it is not available (in which case there is no original method). For python>=2.6 this function would be the original method. There is no confusion.

The problem with hasattr is not a problem since we use it not on a custom class instance but on a module.

Also, it doesn't perform lookup multiple times, since it is called on module import.

[ei-grad]

Handling exception each time on python2.4 definitely would not be the faster way. The try/except block also has its own little overhead, so it would be slower even on python>=2.6.

[sivel]

I'm going to leave this in needs_revision. My requirements for considering this change have been outlined above.

[ei-grad]

@bcoca @sivel I rebased this on current devel, and replaced hasattr by try/except block in import scope. Does it look better for you now?

[abadger]

Sidestepping the whole question of how to make compatibility code... We've just branched stable-2.3 which means that devel is now open for developing Ansible-2.4. Ansible-2.4 drops support for python-2.4 and python-2.5 so we can go back to the code that simply assumes that socket.create_connection is available.

[ansibot]

@ei-grad Greetings! Thanks for taking the time to open this pullrequest. In order for the community to handle your pullrequest effectively, we need a bit more information.

Here are the items we could not find in your description:

• issue type

Please set the description of this pullrequest with this template:
https://raw.githubusercontent.com/ansible/ansible/devel/.github/PULL_REQUEST_TEMPLATE.md

click here for bot help

[mattclay]

#17999 is a simplified fix expected to be in 2.4.2, which does not require Python 2.4 support.