aries-cloudagent-python icon indicating copy to clipboard operation
aries-cloudagent-python copied to clipboard

:arrow_up: Upgrade `aiohttp-apispec` and `apispec`

Open ff137 opened this issue 1 year ago • 7 comments

The aiohttp-apispec project is no longer maintained, and is pinned to some very old versions of apispec and webargs.

It appeared to be the only library that would throw deprecation warnings that had to be added to this pyproject.toml's filterwarnings rules.

I forked the project and manually upgrade some of the dependencies. Not too much effort. All it means is that the version now points to my fork:

aiohttp-apispec = { git = "https://github.com/ff137/aiohttp-apispec.git", tag = "v3.0.1" }

Maybe the maintainer sees my contributions and it can get merged into the official release. Otherwise, anyone can fork my fork, upgrade it, and update the above link.


This permits apispec to go from v3 to v6.6. And webargs to go from v5 to v8.4

This means the custom filterwarning can be removed - the one for ignoring "distutils Version classes are deprecated"

And one more perk is that aiohttp-apispec is now compatible up to python 3.12. So it's one step closer in allowing ACA-Py to finally upgrade from python 3.9 :-)

ff137 avatar Apr 26 '24 13:04 ff137

Everything passed locally. The PR Test failures seem like spurious json-ld context failures. 4 cases of:

FAILED aries_cloudagent/vc/vc_ld/tests/test_manager.py::test_store - pyld.jsonld.JsonLdError: ('Could not expand input before compaction.',)

ff137 avatar Apr 26 '24 14:04 ff137

Sadge. Integration test failures too

ff137 avatar Apr 26 '24 14:04 ff137

I haven't glanced at the specifics just yet but I assume it would be accurate to say that replacing this library with an actively maintained alternative is infeasible?

dbluhm avatar May 08 '24 14:05 dbluhm

@dbluhm I think that would be better, yes. But as far as I could tell, it seemed like quite a bit of refactoring to move to another library.

Maybe it's not such a heavy lift -- it's more that it feels redundant to refactor something that's not broken. If there's other motivation for moving to a more up-to-date library, then that's definitely better. But here the only issue I saw is that it's pinned to some deprecated versions

ff137 avatar May 08 '24 14:05 ff137

Any recent updates on this? I see the PR to aiohttp-apispec has not been merged, so that doesn't seem to be the path.

swcurran avatar May 24 '24 16:05 swcurran

I think we should go ahead with this and look for alternatives in the meantime. It would be better to have a library which is well maintained, but I don't think this is really a downgrade going from what we are using now to @ff137's github.

jamshale avatar May 29 '24 21:05 jamshale

@jamshale agreed - it would be better to use a more well-maintained library, but there's probably not much motivation to replace aiohttp-apispec yet, given other priorities.

As it stands I've just taken the existing aiohttp_apispec repo and upgraded dependencies. I've made a PR to merge to their main repo, and emailed the owner, but haven't gotten any feedback.

So, this will just allow some libraries to upgrade a few major version (like apispec and webargs), and it'll allow for python 3.12 when that's desired.

It probably looks weird to have one dependency pointing to my personal fork, but that can be changed any time, and anyone can fork my fork to build on the existing changes if need be

ff137 avatar May 31 '24 12:05 ff137

3 scenarios failing in integration tests. Each of them due to a validation error:

marshmallow.exceptions.ValidationError: {'wallet_webhook_urls': ['Not a valid list.']}

Seems to be a change in webargs (edit: webargs5 used marshmallow2, and webargs8 uses marshmallow3), which I guess was previously ignoring a bad value being passed: the demo is providing a str when the model expects a List[str]

ff137 avatar May 31 '24 13:05 ff137

Ready to merge?

swcurran avatar May 31 '24 16:05 swcurran