New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Add support for gzip decoding responses #41925

Open
wants to merge 5 commits into
base: devel
from

Conversation

Projects
None yet
3 participants
@sivel
Member

sivel commented Jun 25, 2018

SUMMARY

Add support for gzip decoding responses. Fixes #29670

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

lib/ansible/module_utils/urls.py
lib/ansible/modules/net_tools/basics/uri.py

ANSIBLE VERSION
2.7
ADDITIONAL INFORMATION

@ansibot

This comment has been minimized.

Contributor

ansibot commented Jun 25, 2018

@sivel

This comment has been minimized.

Member

sivel commented Jun 29, 2018

I wonder if we could just import this from xmlrpclib/xmlrpc.client instead?

xmlrpclib should be stdlib, but I know some distros like to make things complicated.

done.

@ansibot

This comment has been minimized.

Contributor

ansibot commented Jun 29, 2018

@sivel

This comment has been minimized.

Member

sivel commented Jun 29, 2018

We could move this logic directly into fetch_url, however that would make things a bit more complicated.

We would have to potentially create our own HTTPResponse subclass.

@ansibot

This comment has been minimized.

Contributor

ansibot commented Jun 29, 2018

The test ansible-test sanity --test import --python 2.6 [explain] failed with 2 errors:

lib/ansible/modules/net_tools/basics/get_url.py:297:0: ImportError: cannot import name GzipDecodedResponse
lib/ansible/modules/net_tools/basics/uri.py:271:0: ImportError: cannot import name GzipDecodedResponse

click here for bot help

@ansibot ansibot added needs_revision and removed core_review labels Jun 29, 2018

@sivel

This comment has been minimized.

Member

sivel commented Jun 29, 2018

Guess there is my answer. py2.6 does not have GzipDecodedResponse.

@sivel

This comment has been minimized.

Member

sivel commented Jun 29, 2018

An example of a Response class that would work here, proving this functionality without the caller of fetch_url having to handle gzip encoding on their own.

diff --git a/lib/ansible/module_utils/urls.py b/lib/ansible/module_utils/urls.py
index d6a51c024c..f2ee7c4817 100644
--- a/lib/ansible/module_utils/urls.py
+++ b/lib/ansible/module_utils/urls.py
@@ -852,6 +852,28 @@ class GzipDecodedResponse(gzip.GzipFile if HAS_GZIP else object):
             self.io.close()


+class Response:
+    def __init__(self, response):
+        self._response = response
+        try:
+            # PY2
+            getheader = self._response.headers.getheader
+        except AttributeError:
+            # PY3
+            getheader = self._response.getheader
+
+        if getheader('content-encoding', '') == 'gzip':
+            self._stream = GzipDecodedResponse(self._response)
+        else:
+            self._stream = self._response
+
+    def __getattr__(self, name):
+        return getattr(self._response, name)
+
+    def read(self, amt=None):
+        return self._stream.read(amt)
+
+
 class Request:
     def __init__(self, headers=None, use_proxy=True, force=False, timeout=10, validate_certs=True,
                  url_username=None, url_password=None, http_agent=None, force_basic_auth=False,
@@ -1264,12 +1286,14 @@ def fetch_url(module, url, data=None, headers=None, method=None,
     r = None
     info = dict(url=url)
     try:
-        r = open_url(url, data=data, headers=headers, method=method,
-                     use_proxy=use_proxy, force=force, last_mod_time=last_mod_time, timeout=timeout,
-                     validate_certs=validate_certs, url_username=username,
-                     url_password=password, http_agent=http_agent, force_basic_auth=force_basic_auth,
-                     follow_redirects=follow_redirects, client_cert=client_cert,
-                     client_key=client_key, cookies=cookies)
+        r = Response(
+                open_url(url, data=data, headers=headers, method=method,
+                         use_proxy=use_proxy, force=force, last_mod_time=last_mod_time, timeout=timeout,
+                         validate_certs=validate_certs, url_username=username,
+                         url_password=password, http_agent=http_agent, force_basic_auth=force_basic_auth,
+                         follow_redirects=follow_redirects, client_cert=client_cert,
+                         client_key=client_key, cookies=cookies)
+        )
         # Lowercase keys, to conform to py2 behavior, so that py3 and py2 are predictable
         info.update(dict((k.lower(), v) for k, v in r.info().items()))

@ansibot ansibot added core_review and removed needs_revision labels Jun 29, 2018

@ansibot ansibot added the stale_ci label Jul 7, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Aug 16, 2018

@sivel sivel force-pushed the sivel:issue/29670 branch to af76c89 Sep 7, 2018

@ansibot ansibot removed the needs_revision label Sep 10, 2018

@sivel sivel changed the title from Add support for gzip decoding responses to [WIP] Add support for gzip decoding responses Sep 17, 2018

@sivel

This comment has been minimized.

Member

sivel commented Sep 17, 2018

I think I probably also need to create a custom HTTPError also, that adds some of the conveniences too.

@ansibot ansibot added the WIP label Sep 17, 2018

@ansibot ansibot added the stale_ci label Sep 25, 2018

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