feature: Add async networking libraries support

[Albo90]

TOP LEVEL FEATURE TASK LIST:

• [x] create stable feature branch feature-branch - async-feature
• [x] support for async networking - aiohttp.Client - client.py, service.py #210
• [x] check httpx package as networking client; should have sync and async API as well. If yes, update tests to have integration testing of requests (sync), aiohttp (async) and httpx (sync, async) packages
• [ ] cover httpx async tests as well
• [ ] documentation
• [ ] integration to master - https://github.com/SAP/python-pyodata/pull/225
• [ ] release

Hi, I used this library recently, it's fantastic very convenient but it doesn't support async pattern. I and my collegue modified the library to support aiohttp, this is the link: https://github.com/Albo90/python-pyodata/tree/async_python If you are interested, we can collaborate to integrate it in your project. We are going to add test and more documentation in a couple of days.

Thanks

[filak-sap]

Impressive. Unfortunately, since you copied all files, I cannot see the needed changes. Can you somehow summarize what needs to be changed?

[Albo90]

Impressive. Unfortunately, since you copied all files, I cannot see the needed changes. Can you somehow summarize what needs to be changed?

Hi filak-sap, we had to create a bit of redundancy for our modifications otherwise it's really difficult for us to integragate async. I try to describe them grouping for file.

async_client

1. Definition of _ new_ method accord to async strategy.
2. Definition of _fetch_metadata with get implementation accord to async.

AsyncService

1. Modification of execute method into OdataHttpRequest, we use "async with" to create request and build response in aim to avoid excessive ridefionition of project structure.
2. Modification of http_get method.

If you want, we can make a reasoning to build other ways of integration

[filak-sap]

Is there a way how to avoid huge code duplications?

[Albo90]

Is there a way how to avoid huge code duplications?

Yes, in my option there is possibility to avoid creation of files (async_client, asyncService) rebuilding the existent methods.

[Albo90]

@filak-sap we published a new version without duplication. The version is in this branch: https://github.com/Albo90/python-pyodata/tree/no_duplication_v1. The library has a little syntax changement when it is used with async (pay attention to method getattr in example).

Sync version:

import requests
from pyodata.client import Client

client = Client('http://example.com', requests)
entities = client.entity_sets.resource.get_entities().execute()
for entity in entities:
      print(entity.attr_name)

print(entities)

Async version:

import asyncio
import aiohttp
from pyodata.client import Client

async def main():
    async with aiohttp.ClientSession() as client:
        service = await Client.build_async_client('http://example.com', client)
        entities = await service.entity_sets.resource.get_entities().select('key').async_execute()
        for entity in entities:
            attr_name = await entity.getattr('attr_name')
            print(attr_name)
        return entities


async_entities = asyncio.run(main())
print(async_entities)

[phanak-sap]

Support of both requests lib and aiohttp lib is definitely a great idea. I had it on my personal backlog for "experiment someday", so nice to see it done and working :) Great job even just as a submitted proposal.

I had read through the issue comments and the commits: https://github.com/Albo90/python-pyodata/commit/543d17535dc5c4a30d38411a14690550f580c4db https://github.com/Albo90/python-pyodata/commit/00ad1dc4feb3b0ca779a496c28794481a5fa8f31 https://github.com/Albo90/python-pyodata/commit/e5b6ec1759917186a4995b9241c57b6fe2afdacb

Few high-level questions:

1. "we can make a reasoning to build other ways of integration" - do you have an alternative idea to compare with the code?
2. There is no new test code in the three commits. Could you pls elaborate how would you like to approach unit testing of this new feature? From what I see, pytest-aiohttp seems to be the choice, but I franky only started few weeks ago to look into the asyncio library itself, so I am not an educated one. Note: I also want to create a dockerized true odata v2/v4 test service as a target for integration testing, but that will take some time and is different use case anyway.
3. Proposal, open for discussion: Why not split the async feature in two distinctive clients modules, since even in the commits the async code is on top of the standard usage? e.g. instead service = await Client.build_async_client for example something like from pyodata.client import AsyncClient or from pyodata.async_client - as a kind of Adapter class for async networking libraries (we should keep pyodata networking library agnostic as an idea, even that we currently are sure only about the requests lib). In the future we will have pyodata.v2.client and pyodata.v4.client probably as well, with lots of shared code across all four combinations of odata V2/V4 + standard/async. My gut feeling is to go for maintainability and be specific for library user what he is actually using and the existing modules will not grow bigger and bigger. Yes, it is like in the first proposal https://github.com/Albo90/python-pyodata/tree/async_python but there the async_service.py was a really true duplicate, with no shared code reused... So I am imagining somehow merge together this module structure and the three commits small code changes. But.. again.. just an idea, perhaps wrong one.

Feature work and Releases

I would at the moment not go into creating PR to master, but rather create new "async" feature branch and do all the needful (code, tests, documentation) there, so pyodata is not stuck in situation where we must release a bugfix with half-baked async feature.. so far there was no need for maintenance branches and we do a flat versioning "from master".

If we could also somehow cover Odata V4 as well, then we could even go in a Pyodata 2.0.0 release with possibility of backward-incompatible changes. Async networking and v2+v4 are big enough features for such version bump.

Feel free to create a PR for code review to such new feature branch.

[Albo90]

Hi @phanak-sap,

1. The expression was about first version when we created async_client and async_service duplicating a lot of code.
2. We are working about test code, we are studying your test set to have a compliance. I think that we will add them in a couple of days.
3. I understand your point of view and we agree about it, but in my opinion to realize it we have to pay attention to position of "execute" method, moving it maybe we have a possibility. We will think about this approach. We stay in touch !!

For PR I haven't permission to create new branch. You should give me permission or create branch for me. Thanks.

[phanak-sap]

Created new feature branch https://github.com/SAP/python-pyodata/tree/async-feature

I have also marked this and odata V4 enhancement at the moment as target for 2.0.0 Milestone, since it is the safest road possible. Maybe will not be needed and will go out in some 1.X version if both features will not be backward-incompatible (which will not force us to maintain 1.x and 2.x for considerable time together).

[liyakun]

Hello @phanak-sap, may I ask whether you have any plan to include the documentation etc. to complete this new feature? Thanks.

[phanak-sap]

Hi @liyakun, yes. But basically all I want is to move the initialization from tests to docs, and still forgetting that :) The feature itself is finished and released, so feel free to try it. Take https://github.com/SAP/python-pyodata/tree/master/tests/integration/networking_libraries as inspiration for initiliazation.

[liyakun]

@phanak-sap thanks a lot, I will have a try.

[phanak-sap]

support tasks finally done.