Add asyncio support
Opened this issue · 6 comments
Currently marathon-python depends has a MarathonClient that depends on requests. Would you accept an AsyncMarathonClient version that uses aiohttp ? I would gladly submit a PR
Gosh. I'm not sure. @EvanKrall what do you think?
I'd be in favor, and would probably work on the PR to Yelp/paasta to use it.
Fortunately, it doesn't look like any of the model objects are doing any lazy fetching of data, so the model objects' API should be able to stay the same, and it looks like just a separate AsyncMarathonClient object would be sufficient.
I'm interested to see how you test the async version. Ideally we'd have mostly the same set of tests for both async and non-async. In general, I care less about DRY in test code than non-test code, so if you end up having to copy-paste-sed the tests, that's fine, but ideally we'd enforce test parity between the two versions somehow.
Great!! I'll keep in mind the test suite during the development of AsyncMarathonClient.
FYI: At B2W we value testing and we use marathon-python on a production environment as a dependency of asgard-api, a flask API that we are currently thinking about moving to asyncio.
btw, the current marathon-python CI build runs only against py2.7 and implementing
AsyncMarathonClient would break compatibility with py2.7. Thats ok ? If so, its ok to support only py>=3.6 ?
It looks like we're testing against py2.7 and py3.3: travis calls make test, which calls tox -e test-py27 and tox -e test-py33.
I'd be okay with bumping the python3 version requirement to 3.6; I don't think I can speak with the same authority for python 2.7 - I don't know how many python 2.7 users use this library. Would it be onerous to put the async client in a different module so that python 2.7 users can still use the requests-based client?