Fix compatibility with webargs 6.0

[sloria]

Follow-up to #176 : fix compatibility with the webargs >=6.0 and remove the version restriction in setup.py.

[c-kruse]

Hey @sloria!

Do you have any direction on what you would like to see this look like? Webargs did a lot of work in 6.0 dropping python 2 and updating their interface - thanks for all of your work on that!

I can see it getting pretty hairy continuing to support python 2, webargs>=0.18.0 along with 6.0+.

[sloria]

I think we can go ahead and drop Python 2 support when flask-apispec supports webargs 6.

[mdantonio]

sloria added the help wanted label on Mar 1

Hello @sloria, can I ask you which kind of help is wanted here? Can we help in any way with this issue?

The upgrade to webargs 6.0+ would be veeeery appreciated :-)

[sloria]

A PR that supports webargs>=6 and drops support for Python 2 would be much appreciated. Even a WIP PR would help, as others can always continue on any partial work.

[mdantonio]

I could try, but starting by analyzing the #176 issue it seems to me that a decision about a major change is needed... And I don't have the authority to do that.

core flaskparser.parse before 6.0:

    :param tuple locations: Where on the request to search for values.
        Can include one or more of ``('json', 'querystring', 'form',
        'headers', 'cookies', 'files')``.

core flaskparser.parse from 6.0:

    :param str location: Where on the request to load values.
        Can be any of the values in :py:attr:`~__location_map__`. By
        default, that means one of ``('json', 'query', 'querystring',
        'form', 'headers', 'cookies', 'files', 'json_or_form')``.

i.e. the parameter changed from a tuple to a single value

By changing wrapper.py

from:

parsed = parser.parse(schema, locations=option['kwargs']['locations'])

to, let's say something like this:

parsed = parser.parse(schema, location=option['kwargs']['locations'][0])

or:

parsed = parser.parse(schema, location=option['kwargs']['location'])

webargs 6.0+ (tested with 6.1.0) seems to work... but in both cases this is a major change because this:

use_kwargs({"xxx": fields.XXX}, locations=["json", "query"])

will no longer work as expected, i.e. 'query' will be ignored (first very rough fix) or the whole locations will be ignored at all (second fix)

With the second fix it should be changed in:

use_kwargs({"xxx": fields.XXX}, location="json")

and multiple locations will no longer be supported

If you can give any hint about the better direction to take for this issue, I may try a PR.

Maybe some warning like this?

if option['kwargs'].get('locations'):
  warning("locations parameters is no longer supported, please use the new location parameter with a single value")
  parsed = parser.parse(schema, location=option['kwargs']['locations'][0])
else:
  parsed = parser.parse(schema, location=option['kwargs']['location'])

[sloria]

Given how flask-apispec is closely coupled with webargs, perhaps we should only allow location and release a new major version. I don't really want to add the maintenance burden of supporting multiple webargs release lines.

[mdantonio]

Ok, two more things.

the requirement will become webargs>=6.0.0, because by changing locations with location the wrapper will no longer work with webargs 5.x

and, most important: tests like this
https://github.com/jmcarp/flask-apispec/blob/master/tests/test_views.py#L14-L20

will fail because the default locations before was: ("querystring", "form", "json") and now it is "json" but for GET methods a querystring is expected

This means that several use_kwargs that now work without specifying any locations thanks to the wide default, will stop to work and will required an explicit location='something' to be specified. For huge projects this can be very very cumbersome / frustrating

If I have your blessing I will work on that PR, by also fixing the tests by specifying a location where missing

[sloria]

Indeed, it is a breaking change that affected a lot of usage, but it really was the right course. See marshmallow-code/webargs#420 and the linked issues for further context on the decision.

If I have your blessing I will work on that PR, by also fixing the tests by specifying a location where missing

Sounds good--thanks for doing this!

[mdantonio]

First PR is ready.

Changed the locations parameter in use_kwargs to reflect changes in webargs 6.0.0+ => locations:tuple -> location:str

Fixed tests to add a location when missing (due to default location changed in webargs, now an explicit location is required more than before)

Dropped compatibility with python2 (removed six, conditional requirements in setup, replaced Classifiers with python_requires, removed # -- coding: utf-8 --, and other small changes).

Open issue: I'm unable to make 4 tests to work, all have multiple use_kwargs. At the moment these tests are commented in test_views.py (Added a "# I'M UNABLE TO MAKE THIS WORK" comment to mark them) to let other tests on complete.

[mdantonio]

Open issue: I'm unable to make 4 tests to work, all have multiple use_kwargs. At the moment these tests are commented in test_views.py (Added a "# I'M UNABLE TO MAKE THIS WORK" comment to mark them) to let other tests on complete.

Just to update the issue and reflect latest changes in the PR (#195): all tests are now working as expected (the problem was due to the multiple schemas raising unknown fields, it is now solved by setting unknown=EXCLUDE)

The PR also bumped the required marshmallow version from >=2.0.0 to >=3.0.0

[sloria]

Thanks again for the PR! 0.10.0 is released

[mdantonio]

You are welcome, later today I will upgrade flask-apispec in my projects for more extensive tests