inveniosoftware / invenio-files-rest

REST API for uploading/downloading files for Invenio.
https://invenio-files-rest.readthedocs.io
MIT License
9 stars 68 forks source link

views: fix compatibility with webargs 5.x #185

Closed sloria closed 5 years ago

sloria commented 5 years ago

Changed the default arguments for args that are both optional and don't define missing from None to marshmallow.missing.

I verified that the tests are passing locally with the following webargs installations:

see #184

sloria commented 5 years ago

Tests are failing on Python 2.7 with webargs 5 because webargs 5 requires simplejson to be installed on Python 2. webargs conditionally installs simplejson on Python 2, so I'm not quite sure why it's not getting installed on Travis. Perhaps it has something to do with how requirements-builder is parsing install_requires?

I don't really have more time to dig into it right now. Maintainers: feel free to fix up this PR as you see fit.

ntarocco commented 5 years ago

@sloria I have investigated and it looks like it is more related to pip when installing from pypi.

Installes packaged when installing from `pypi`, no `simplejson`. ```bash $ vf new -p python2.7 temp Running virtualenv with interpreter /usr/local/bin/python2.7 New python executable in /Users/ntarocco/.virtualenvs/temp/bin/python2.7 Also creating executable in /Users/ntarocco/.virtualenvs/temp/bin/python Installing setuptools, pip, wheel... done. $ pip install webargs DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7. Collecting webargs Using cached https://files.pythonhosted.org/packages/7f/9b/d99025e21c062f4520905c75e14681bff4047255893a0e3c7c2b28b37e92/webargs-5.1.0-py2.py3-none-any.whl Collecting marshmallow>=2.15.2 (from webargs) Using cached https://files.pythonhosted.org/packages/9a/3c/4cc463c53136dc503f9ee234c4a6592e1c7411cb362f844e80df70361f29/marshmallow-2.18.0-py2.py3-none-any.whl Installing collected packages: marshmallow, webargs Successfully installed marshmallow-2.18.0 webargs-5.1.0 $ pip freeze DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7. marshmallow==2.18.0 webargs==5.1.0 ```
Installed packages when installing locally, `simplejson` appears. ```bash $ git clone git@github.com:marshmallow-code/webargs.git Cloning into 'webargs'... cd webargsremote: Enumerating objects: 50, done. remote: Counting objects: 100% (50/50), done. remote: Compressing objects: 100% (37/37), done. remote: Total 4621 (delta 18), reused 32 (delta 13), pack-reused 4571 Receiving objects: 100% (4621/4621), 2.22 MiB | 1.35 MiB/s, done. Resolving deltas: 100% (3314/3314), done. $ git co 5.1.0 Note: checking out '5.1.0'. You are in 'detached HEAD' state. You can look around, make experimental changes and commit them, and you can discard any commits you make in this state without impacting any branches by performing another checkout. If you want to create a new branch to retain commits you create, you may do so (now or later) by using -b with the checkout command again. Example: git checkout -b HEAD is now at f12c0b8... Bump version and update changelog $ vf new -p python2.7 temp Running virtualenv with interpreter /usr/local/bin/python2.7 New python executable in /Users/ntarocco/.virtualenvs/temp/bin/python2.7 Also creating executable in /Users/ntarocco/.virtualenvs/temp/bin/python Installing setuptools, pip, wheel... done. $ pip install -e .[all] DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7. Obtaining file:///Users/ntarocco/workspace/python/webargs webargs 5.1.0 does not provide the extra 'all' Collecting marshmallow>=2.15.2 (from webargs==5.1.0) Using cached https://files.pythonhosted.org/packages/9a/3c/4cc463c53136dc503f9ee234c4a6592e1c7411cb362f844e80df70361f29/marshmallow-2.18.0-py2.py3-none-any.whl Collecting simplejson>=2.1.0 (from webargs==5.1.0) Using cached https://files.pythonhosted.org/packages/d9/52/3dd4aed32371fc2aa0e14f68fd7563e671336f4df3982246e87dd87bffc2/simplejson-3.16.0-cp27-cp27m-macosx_10_12_x86_64.whl Installing collected packages: marshmallow, simplejson, webargs Running setup.py develop for webargs Successfully installed marshmallow-2.18.0 simplejson-3.16.0 webargs $ pip freeze DEPRECATION: Python 2.7 will reach the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 won't be maintained after that date. A future version of pip will drop support for Python 2.7. marshmallow==2.18.0 simplejson==3.16.0 -e git+git@github.com:marshmallow-code/webargs.git@f12c0b8f66c0a86cc15e90b322162e15450cc952#egg=webargs ```

Can you reproduce the same?

sloria commented 5 years ago

Ah, I think the problem is that webargs is released as a universal (Python 2 and 3 compatible) wheel. The wheel doesn't contain simplejson as a dependency. I've opened an issue on webargs: https://github.com/marshmallow-code/webargs/issues/363 .

One workaround for the time being would be to explicitly install simplejson.

sloria commented 5 years ago

OK, I added simplejson as an explicit requirement, which fixes Python 2 behavior. This should be good to go now.

ntarocco commented 5 years ago

@sloria great thanks! We have just to check a couple of things on our side and then we will merge! Thanks again for this contribution, very appreciated!

sloria commented 5 years ago

Sounds good @ntarocco . I'm happy to make any more changes, but I'd appreciate if the maintainers went ahead and made any minor nit-fixing to get this merged--no need to block on me =).

sloria commented 5 years ago

FWIW, I just released webargs 5.1.1 which fixes the simplejson installation issue. So explicitly installing simplejson isn't strictly necessary. Doesn't hurt to be explicit, though. I'll leave it up to the maintainers to decide.

lnielsen commented 5 years ago

We're going to change the license of Invenio-Files-REST in 2 weeks time to MIT License. Can you just confirm that you're ok with licensing your contribution under MIT License instead of GPL.

sloria commented 5 years ago

Confirmed. I am good with the license change.