KeyError with file.managed HTTPS source

[anlutro]

State:

phpmyadmin-archive:
  file.managed:
    - name: /usr/share/phpmyadmin-{{ cfg.version }}.tar.xz
    - source: https://files.phpmyadmin.net/phpMyAdmin/{{ cfg.version }}/phpMyAdmin-{{ cfg.version }}-english.tar.xz
    - source_hash: https://files.phpmyadmin.net/phpMyAdmin/{{ cfg.version }}/phpMyAdmin-{{ cfg.version }}-english.tar.xz.sha1

Relevant part from the log:

[DEBUG   ] Requesting URL https://files.phpmyadmin.net/phpMyAdmin/4.4.11/phpMyAdmin-4.4.11-english.tar.xz.sha1 using GET method
[DEBUG   ] Traceback (most recent call last):
  File "/tmp/.vagrant_740f1b_salt/salt/states/file.py", line 1486, in managed
    **kwargs
  File "/tmp/.vagrant_740f1b_salt/salt/modules/file.py", line 3329, in get_managed
    hash_fn = __salt__['cp.cache_file'](source_hash, saltenv)
  File "/tmp/.vagrant_740f1b_salt/salt/modules/cp.py", line 365, in cache_file
    result = __context__['cp.fileclient'].cache_file(path, saltenv)
  File "/tmp/.vagrant_740f1b_salt/salt/fileclient.py", line 157, in cache_file
    return self.get_url(path, '', True, saltenv)
  File "/tmp/.vagrant_740f1b_salt/salt/fileclient.py", line 625, in get_url
    destfp.write(query['text'])
KeyError: 'text'

[ERROR   ] Unable to manage file: 'text'

Versionreport:

$ salt-ssh --versions-report
Salt Version:
           Salt: 2015.8.1-183-gcfe39df

Dependency Versions:
         Jinja2: 2.8
       M2Crypto: Not Installed
           Mako: Not Installed
         PyYAML: 3.11
          PyZMQ: 14.7.0
         Python: 2.7.10 (default, Sep 13 2015, 20:30:50)
           RAET: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.2
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
        libnacl: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
   python-gnupg: Not Installed
          smmap: Not Installed
        timelib: Not Installed

System Versions:
           dist: debian stretch/sid 
        machine: x86_64
        release: 4.2.0-1-amd64
         system: debian stretch/sid

[anlutro]

Looks like it might be because the sha1 file is served with Content-Type: application/octet-stream as opposed to text/plain.

This PR seems to have introduced the error: #28065 (develop) #28066 (2015.8)

[cachedout]

@anlutro I'm just holding that PR for another day to see if @jacksontj pops by to offer any insight. If not, I'll merge it and we can close this. Thanks for your patience.

[anlutro]

Sure, no problem.

On Tue, 3 Nov 2015 16:39 Mike Place notifications@github.com wrote:

@anlutro https://github.com/anlutro I'm just holding that PR for
another day to see if @jacksontj https://github.com/jacksontj pops by
to offer any insight. If not, I'll merge it and we can close this. Thanks
for your patience.

—
Reply to this email directly or view it on GitHub
#28477 (comment).

[jacksontj]

Another one... sigh. Well I just submitted #28573 with an attempt to fix this whole situation. The problem is that salt.utils.http has a really... complicated interface ;) The lack of tests on it makes is complicated to fix stuff-- since more often than not we break something else. I have an issue open elsewhere to rewrite/simplify/remove that library, but for now we'll continue whack-a-mole :/

[cachedout]

@jacksontj Does #28573 supercede this PR or were you thinking they would both be merged?

[jacksontj]

Oh, I didn't even see the other PR (sorry about that). From looking at it, I think my PR would probably cover this better. The eternal problem we're running into here is that all the HTTP libraries have very different interfaces so ret['handle'].body doesn't work for all clients-- my PR attempts to normalize them all into ret['body'].

I tested locally with requests and tornado, so we should be covered? :/

[cachedout]

If @anlutro is fine with it then, I'll close his and merge yours when the test finishes. Thanks @jacksontj

[anlutro]

Seems fine to me! I'll be able to verify that it works with my setup
tomorrow.

On Wed, 4 Nov 2015 17:37 Mike Place notifications@github.com wrote:

If @anlutro https://github.com/anlutro is fine with it then, I'll close
his and merge yours when the test finishes. Thanks @jacksontj
https://github.com/jacksontj

—
Reply to this email directly or view it on GitHub
#28477 (comment).

[anlutro]

I don't have requests or tornado. My issue was with a sha1 hash file that
was served with a non plain text content type. An issue should be linked in
my pull request.

On Wed, 4 Nov 2015 17:46 Thomas Jackson notifications@github.com wrote:

Oh, I didn't even see the other PR (sorry about that). From looking at it,
I think my PR would probably cover this better. The eternal problem we're
running into here is that all the HTTP libraries have very different
interfaces so ret['handle'].body doesn't work for all clients-- my PR
attempts to normalize them all into ret['body'].

I tested locally with requests and tornado, so we should be covered? :/

—
Reply to this email directly or view it on GitHub
#28477 (comment).

[anlutro]

Seems to work fine now 👍