webob-1.8.1 breaks projects

[Matthew Thode]

Stuff like this

ft2.2: glance.tests.unit.common.test_wsgi.ResourceTest.test_translate_exception_StringException: Traceback (most recent call last):
  File "/home/zuul/src/git.openstack.org/openstack/glance/.tox/py27/local/lib/python2.7/site-packages/mock/mock.py", line 1297, in patched
    arg = patching.__enter__()
  File "/home/zuul/src/git.openstack.org/openstack/glance/.tox/py27/local/lib/python2.7/site-packages/mock/mock.py", line 1369, in __enter__
    original, local = self.get_original()
  File "/home/zuul/src/git.openstack.org/openstack/glance/.tox/py27/local/lib/python2.7/site-packages/mock/mock.py", line 1343, in get_original
    "%s does not have the attribute %r" % (target, name)
AttributeError: <class 'webob.acceptparse.AcceptLanguage'> does not have the attribute 'best_match'

[Matthew Thode (prometheanfire)]

Found in this review, see patchset 4's zuul failures in the cross tests for details.

https://review.openstack.org/562351

[Chris Dent (cdent)]

As other people have probably noticed the accept handing in webob 1.8.x has completely changed [1] and even though there are some methods that are preserved but deprecated (best_match) they don't behave in the same way that they used to because the underlying implementation is much different.

I've tried to figure out what the proper new way to do accept-language handling (in the nova/tests/unit/api/openstack/test_wsgi.py tests, for example) but thus far I've not figured it out. This is probably because I'm also trying to have my dinner. I'll have dinner, a sleep, and come back again tomorrow, but wanted to dump some state.

[1] https://docs.pylonsproject.org/projects/webob/en/stable/whatsnew-1.8.html#backwards-incompatibilities

[Brian Rosmaita (brian-rosmaita)]

Glance: failures for both py27 and py35, same tests in each:

glance.tests.unit.common.test_wsgi.ResourceTest.test_translate_exception
glance.tests.unit.common.test_wsgi.RequestTest.test_best_match_language_unknown

Traceback looks like what Matthew posted above.

[Chris Dent (cdent)]

In the nova `test_wsgi` tests there are two different issues to be dealt with:

* short term: the new webob is more strict when parsing qvalues (the q=$foo bits in the header). If these are a fractional value less than 1, the _must_ have a leading 0. That 0.5 instead of .5.

* long term: the best_match method used in best_match_language on nova's Request object is deprecated. The new alternatives are either `lookup` or `basic_filtering` depending on the results desired.

Patch for the short term forthcoming.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/564255

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/564255
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=5b00f19109b5f4d2ae229f7e8d79a4c5f0a66492
Submitter: Zuul
Branch: master

commit 5b00f19109b5f4d2ae229f7e8d79a4c5f0a66492
Author: Chris Dent <email address hidden>
Date: Wed Apr 25 16:26:53 2018 +0100

    Make accept-language tests work with webob 1.8.x

    webob 1.8.x has altered its processing for accept-* headers [1],
    including being less accomodating of less than well-formed qvalues.
    If it is unable to parse a header, rather than raising an error of
    anything like that, a special class of header is created instead.
    This header still responds on all the same methods as before, but with
    different results. In the big picture this is a good thing (for
    correcting processing for content-negotiation) but it is a bit weird
    for us.

    This change fixes the immediate short term problem, broken unit tests,
    by fixing their bad qvalues. The new values will continue to work with
    old webob. We can hope/assume that most clients will send well-formed
    qvalues.

    Longer term we will need to address how the nova Request object is
    choosing to do best match handling on accept headers to address that
    webob has deprecated (with a long warning period) methods used by the
    object.

    Partial-Bug: #1765748

    [1] https://docs.pylonsproject.org/projects/webob/en/stable/whatsnew-1.8.html#backwards-incompatibilities

    Change-Id: I2034d30cc8d9354be80d39e05b8488cb99c32ecf

[Brian Rosmaita (brian-rosmaita)]

Glance doesn't have any specific q-value tests broken at this point (we have broken tests, just not those). Like Nova, we have a glance Request object that relies on the best_match() that is being eliminated by webob. Using the webob replacement lookup() method doesn't make our tests pass out of the box. The different behavior has to do with the circumstances under which None is returned by the glance Request object's best_match_language() method.

The webob lookup() takes two optional arguments, default_tag and default, and both cannot be none. The difference is if there's no match by the algorithm, default is returned. If default_tag has a value, then *it* will be returned ... unless it is specifically ruled out in the header string by being mentioned with q=0 (in which case, default (which could be None in this case) is returned).

So we can't use lookup() without specifying a value for one of default_tag or default, which is going to mean there will be circumstances under which None is not returned for lookup() whereas it was for best_match(). We could do some monkeying around in best_match_language() to preserve current behavior, e.g., use a sentinel value for default and then return None if lookup() returns the sentinel. Not sure what the best course of action is here. Just making notes so I don't forget I thought about this.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/564883

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/564883
Committed: https://git.openstack.org/cgit/openstack/glance/commit/?id=73cc41c9ceb4e38b60ea22402524bae49aa41c32
Submitter: Zuul
Branch: master

commit 73cc41c9ceb4e38b60ea22402524bae49aa41c32
Author: Brian Rosmaita <email address hidden>
Date: Fri Apr 27 16:46:14 2018 -0400

    Prepare for WebOb 1.8.1

    WebOb 1.8.1 removes a function (deprecated for a long time) that
    Glance uses for negotiating 'Accept-Language' headers. The removed
    function is replaced by a function that implements the "Lookup"
    scheme defined in RFC 4647.

    Glance wraps the webob.Request object and provides a convenience
    method around the old function. This patch uses the new webob
    function in the convenience method and wraps it in such a way that
    the convenience method preserves its current behavior (at least with
    respect to the Glance tests). But ... "Lookup" is compliant with RFC
    7231, whereas the old function was not. This means that given
    sufficiently complex Accept-Language header values (containing
    q-values), the best match under webob 1.8.1 will be different from
    the best match under webob 1.7.x. Given that the best match will,
    however, be RFC 7231 compliant, this is an acceptable change.

    One further complication is that the lookup() function is not
    available in webob 1.7.x. Thus this patch handles both versions. I
    suggest that as soon as WebOb===1.8.1 is merged into
    lower_constraints, the code be simplified to handle 1.8.x only, and
    the following release note be submitted with that patch:

    Negotiation of the 'Accept-Language' header now follows the "Lookup"
    matching scheme described in `RFC 4647, section 3.4
    <https://tools.ietf.org/html/rfc4647.html#section-3.4>`_. The
    "Lookup" scheme is one of the algorithms suggested in `RFC 7231,
    section 5.3.5
    <https://tools.ietf.org/html/rfc7231.html#section-5.3.5>`_. (This is
    due to a change in an underlying library, which previously used a
    matching scheme that did not conform to `RFC 7231
    <https://tools.ietf.org/html/rfc7231.html>`_.)

    Change-Id: Ib9735b394a134de9a53d577b4d8b5a9c760b7780
    Closes-bug: #1765748

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/568304

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/568304
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=f37895dc59f7bc2127b4e1ba8e38ab50e3f1f9cb
Submitter: Zuul
Branch: master

commit f37895dc59f7bc2127b4e1ba8e38ab50e3f1f9cb
Author: Lance Bragstad <email address hidden>
Date: Mon May 14 14:29:52 2018 +0000

    Update tests to work with WebOb 1.8.1

    WebOb recently changed how they validate Accept-* headers [0] to
    closely follow the definitions in RFC 7231 Section 5.3.2 [1].

    WebOb now checks the actual value of the Accept-Language header and
    compares it to a regular expression. This caused a couple keystone
    unit tests to fail because we were generating random UUIDs for
    Accept-Language testing just to make sure the logic within keystone
    was behaving properly. The UUID format actual fails the new regular
    expression being used by WebOb to validate the Accept-Language
    header.

    This commit changes the tests to use a language header that passes
    the new validation put in place by WebOb.

    [0] https://docs.pylonsproject.org/projects/webob/en/stable/whatsnew-1.8.html#backwards-incompatibilities
    [1] https://tools.ietf.org/html/rfc7231.html#section-5.3.2

    Change-Id: Ic53f4d00bc6c8dec08ec1bff589a91ff359276e1
    Closes-Bug: 1765748

[Matthew Thode (prometheanfire)]

python-zvm-sdk is now holding openstack back from moving forward on this and finally closing it. They had a commit blocking >=1.8.0 but no reasoning given.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/keystone 14.0.0.0b2 development milestone.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/glance 17.0.0.0b2 development milestone.