[elbe-devel] [PATCH] soapclient: download_file: fix uninitialized retry

[Manuel Traut]

-- 
------------------------------------------------
Linutronix GmbH | Bahnhofstraße 3 | D-88690 Uhldingen-Mühlhofen
Phone: +49 7556 25 999 16; Fax.: +49 7556 25 999 99

Hinweise zum Datenschutz finden Sie hier
(Informations on data privacy can be found here):
https://linutronix.de/kontakt/Datenschutz.php

Linutronix GmbH | Firmensitz (Registered Office): Uhldingen-Mühlhofen
Registergericht (Registration Court): Amtsgericht Freiburg i.Br., HRB700 806
Geschäftsführer (Managing Directors): Heinz Egger, Thomas Gleixner
On Wed, Sep 05, 2018 at 07:58:01PM +0200, John Ogness wrote:
> On 2018-09-05, Kurt Kanzenbach <kurt at linutronix.de> wrote:
> >> I do not understand why retrying the download would help (in my case
> >> the elbe-daemon crashed), but I chose to initialize retry so that it
> >> retries once.
> >
> > If retry doesn't work at all, why not remove the retry handling
> > completely?
> 
> As I mentioned, I don't understand why the retry logic was implemented
> in the first place. I assume there must be some reason. (??)

--8<--
commit 740690f56b0d79076face12ac365a18bde70af76
Author: Manuel Traut <manut at linutronix.de>
Date:   Mon Mar 7 15:39:29 2016 +0100

    soapclient: make file download more robust
    
    add a retry functionality and print a readable error, if the file can't
    be transfered
    
    Traceback (most recent call last):
      File "/usr/bin/elbe", line 61, in <module>
        cmdmod.run_command( sys.argv[2:] )
      File "/usr/lib/python2.7/dist-packages/elbepack/commands/control.py", line 106, in run_command
        action.execute (control, opt, args[1:])
      File "/usr/lib/python2.7/dist-packages/elbepack/soapclient.py", line 355, in execute
        client.download_file (builddir, f.name, dst_fname)
      File "/usr/lib/python2.7/dist-packages/elbepack/soapclient.py", line 85, in download_file
        ret = self.service.get_file (builddir, filename, part)
      File "/usr/lib/python2.7/dist-packages/suds/client.py", line 566, in __call__
        return client.invoke(args, kwargs)
      File "/usr/lib/python2.7/dist-packages/suds/client.py", line 705, in invoke
        result = self.send(soapenv)
      File "/usr/lib/python2.7/dist-packages/suds/client.py", line 747, in send
        reply = self.options.transport.send(request)
      File "/usr/lib/python2.7/dist-packages/suds/transport/https.py", line 66, in send
        return HttpTransport.send(self, request)
      File "/usr/lib/python2.7/dist-packages/suds/transport/http.py", line 80, in send
        fp = self.u2open(u2request)
      File "/usr/lib/python2.7/dist-packages/suds/transport/http.py", line 127, in u2open
        return url.open(u2request, timeout=tm)
      File "/usr/lib/python2.7/urllib2.py", line 431, in open
        response = self._open(req, data)
      File "/usr/lib/python2.7/urllib2.py", line 449, in _open
        '_open', req)
      File "/usr/lib/python2.7/urllib2.py", line 409, in _call_chain
        result = func(*args)
      File "/usr/lib/python2.7/urllib2.py", line 1227, in http_open
        return self.do_open(httplib.HTTPConnection, req)
      File "/usr/lib/python2.7/urllib2.py", line 1200, in do_open
        r = h.getresponse(buffering=True)
      File "/usr/lib/python2.7/httplib.py", line 1136, in getresponse
        response.begin()
      File "/usr/lib/python2.7/httplib.py", line 453, in begin
        version, status, reason = self._read_status()
      File "/usr/lib/python2.7/httplib.py", line 417, in _read_status
        raise BadStatusLine(line)
    httplib.BadStatusLine: ''
    elbe control get_files Failed
    Giving up
    
    Signed-off-by: Manuel Traut <manut at linutronix.de>
--8<--

> I vote to remove the retry logic.

there was already a try to do so, but it was also bad, because it introduced
the uninitialized variable issue:

--8<--
commit 6b3fb9d01f170e708f357ea6c5f2b0e356ef0f3d
Author: Manuel Traut <manut at linutronix.de>
Date:   Fri Apr 8 17:29:34 2016 +0200

    soapclient: remove endless loop in download_file

    the retry logic was very braindead :(

    Signed-off-by: Manuel Traut <manut at linutronix.de>
--8<--

My past commit messages were bad. But if i remember it correctly, we saw the
BadStatusLine Exception several times. The documentation about BadStatusLine
is:


--8<--
exception httplib.BadStatusLine

    A subclass of HTTPException. Raised if a server responds with a HTTP status code that we don’t understand.

    New in version 2.0.
--8<--

So i would prefer to print as much information about the exception as possible.
We should keep and annotate the retry logic, as 'FIXME: retry should be removed
if the problem no longer occurs, or is solved in another way'

If this is ok for you, i will come up with a v2 of the patch. (I also posted
this as part of the pylint cleanup series)

  Manu

> John Ogness
> 
> _______________________________________________
> elbe-devel mailing list
> elbe-devel at linutronix.de
> https://lists.linutronix.de/mailman/listinfo/elbe-devel