Bug #8104

[SDK] PyCurl 7.21.5 breaks keepclient

Added by Peter Amstutz over 1 year ago. Updated 4 months ago.

Status:ResolvedStart date:04/12/2017
Priority:NormalDue date:
Assignee:Tom Clegg% Done:

0%

Category:SDKs
Target version:2017-04-12 sprint
Story points-Remaining (hours)0.00 hour
Velocity based estimate-

Description

Symptom: keepclient fails with TypeError: _socket_open() takes at least 3 arguments (2 given)

This is due to an API change in PyCurl:

From http://pycurl.sourceforge.net/ChangeLog

Version 7.21.5 [requires libcurl-7.19.0 or better] - 2016-01-05
        * Breaking change: OPENSOCKETFUNCTION callback API now mirrors that
          of libcurl:
          1. The callback now takes two arguments, `purpose' and `address`.
             Previously the callback took `family', `socktype', `protocol`
             and `addr' arguments.
          2. The second argument to the callback, `address', is a
             `namedtuple' with `family', `socktype', `protocol' and
             `addr' fields.
          3. `addr' field on `address' for AF_INET6 family addresses is a
             4-tuple of (address, port, flow info, scope id) which matches
             Python's `socket.getaddrinfo' API.

          It seems that libcurl may mishandle error return from an
          opensocket callback, as would happen when code written for
          pre-PycURL 7.21.5 API is run with PycURL 7.21.5 or newer,
          resulting in the application hanging.

As a quick fix, we can add a version constraint <7.21.5 to prevent installing the newest PyCurl package.


Subtasks

Task #11430: Review 8104-pycurl-721Resolved


Related issues

Related to Arvados - Bug #8120: keep error, TypeError: _socket_open() takes at least 3 ar... Resolved 01/06/2016
Related to Arvados - Bug #11237: python-arvados-python-client depends on old versions of p... Resolved 03/09/2017

Associated revisions

Revision 722e1477
Added by Peter Amstutz over 1 year ago

Add pycurl version constraint to Python SDK because of breaking API change, refs #8104

Revision fff5527a
Added by Brett Smith over 1 year ago

8104: Pin the pycurl backport version.

Refs #8104.

Revision fff5527a
Added by Brett Smith over 1 year ago

8104: Pin the pycurl backport version.

Refs #8104.

Revision 9dae3e6d
Added by Tom Clegg 5 months ago

Merge branch '8104-pycurl-721'

closes #8104

History

#1 Updated by Peter Amstutz over 1 year ago

  • Description updated (diff)

#2 Updated by Peter Amstutz 5 months ago

Suggest the following version dependency change:

-          'pycurl >=7.19.5.1, <7.21.5',
+          'pycurl >=7.19.3',

Due to the API change, we need to detect the pycurl version and use the correct API.

Specifying >=7.19.3 allows us to depend on OS-provided packages on Debian 8, Ubuntu 14.04, Ubuntu 16.04.

Will need to provide backport for Centos 7 (which ships 7.19.0 which we can't use due to #6073).

Because pycurl 7.21.5 has a breaking API change, we should continue backport 7.19.5.3 to avoid breaking anything else that might depend on pycurl.

Debian 7, Centos 6, and Ubuntu 12.04 would also require backports, but they are EOL.

#3 Updated by Tom Clegg 5 months ago

If this happens before #11308 merges, please consider cherry-picking adac4b967382c280022d202d56d7fee41764ef35 first.

#4 Updated by Peter Amstutz 5 months ago

(I think this is the minimum version that has the #6073 fix: https://github.com/pycurl/pycurl/blob/master/ChangeLog#L473)

#5 Updated by Peter Amstutz 5 months ago

  • Target version set to 2017-04-12 sprint

#6 Updated by Tom Clegg 5 months ago

  • Category set to SDKs
  • Status changed from New to In Progress
  • Assignee set to Tom Clegg

#8 Updated by Tom Clegg 5 months ago

Amended: stop building pycurl deb/rpm packages, except for ubuntu1204 and centos7.

8104-pycurl-721 @ 625745e03fd6aef5cae69b8a603d808d8b1e7388

#9 Updated by Peter Amstutz 5 months ago

-          'pycurl >=7.19.5.1, <7.21.5',
+          'pycurl >=7.19.5.1',

Needs to be 'pycurl >=7.19.3', see note-2

#10 Updated by Peter Amstutz 5 months ago

(On further discussion, disregard previous note)

What's the reasoning here? Explicitly closing the socket after every request seems like it will defeat HTTP keepalive.

                    try:
                        curl.perform()
                    except Exception as e:
                        raise arvados.errors.HttpError(0, str(e))
                    finally:
                        if self._socket:
                            self._socket.close()
                            self._socket = None

#11 Updated by Tom Clegg 5 months ago

  • Status changed from In Progress to Resolved
  • % Done changed from 0 to 100

Applied in changeset arvados|commit:9dae3e6dde2993bc44e90539141b442c899cf114.

Also available in: Atom PDF