You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feature: Add async networking libraries support #208
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
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.
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
Definition of _ new_ method accord to async strategy.
Definition of _fetch_metadata with get implementation accord to async.
AsyncService
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.
Modification of http_get method.
If you want, we can make a reasoning to build other ways of integration
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.
"we can make a reasoning to build other ways of integration" - do you have an alternative idea to compare with the code?
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.
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.
The expression was about first version when we created async_client and async_service duplicating a lot of code.
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.
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.
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).
Activity
filak-sap commentedon Mar 8, 2022
Impressive. Unfortunately, since you copied all files, I cannot see the needed changes. Can you somehow summarize what needs to be changed?
Albo90 commentedon Mar 8, 2022
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
AsyncService
If you want, we can make a reasoning to build other ways of integration
filak-sap commentedon Mar 8, 2022
Is there a way how to avoid huge code duplications?
Albo90 commentedon Mar 8, 2022
Yes, in my option there is possibility to avoid creation of files (async_client, asyncService) rebuilding the existent methods.
Albo90 commentedon Mar 8, 2022
@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:
Async version:
phanak-sap commentedon Mar 9, 2022
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:
Albo90@543d175
Albo90@00ad1dc
Albo90@e5b6ec1
Few high-level questions:
pytest-aiohttpseems 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.service = await Client.build_async_clientfor example something likefrom pyodata.client import AsyncClientor frompyodata.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 therequestslib). 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 theasync_service.pywas 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 commentedon Mar 9, 2022
Hi @phanak-sap,
For PR I haven't permission to create new branch. You should give me permission or create branch for me. Thanks.
phanak-sap commentedon Mar 9, 2022
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).
[-]Add support async Http library[/-][+]Add async networking libraries support [/+][-]Add async networking libraries support [/-][+]feature: Add async networking libraries support [/+]liyakun commentedon May 31, 2023
Hello @phanak-sap, may I ask whether you have any plan to include the documentation etc. to complete this new feature? Thanks.
phanak-sap commentedon May 31, 2023
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 commentedon May 31, 2023
@phanak-sap thanks a lot, I will have a try.
phanak-sap commentedon Dec 19, 2023
support tasks finally done.