Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create aiohttp.ClientSession once per application rather than per request #93

Open
dmitryashutov opened this issue Dec 15, 2022 · 2 comments

Comments

@dmitryashutov
Copy link
Member

In the AsyncClient._do_request we create the aiohttp.ClientSession instance for every single request (

async with aiohttp.ClientSession(headers=headers) as session:
)

Maybe it's not a big problem but the aiohttp documentation says:

Don’t create a session per request. Most likely you need a session per application which performs all requests altogether.
More complex cases may require a session per site, e.g. one for Github and other one for Facebook APIs. Anyway making a session for every request is a very bad idea.
A session contains a connection pool inside. Connection reusage and keep-alives (both are on by default) may speed up total performance.

It makes sense to follow the recommendation above and provide the API for closing the session during the application cleanup.

@mkizesov
Copy link
Contributor

Also we need to keep in mind this https://docs.aiohttp.org/en/stable/client_advanced.html#graceful-shutdown

dmitryashutov added a commit to dmitryashutov/fhir-py that referenced this issue Jan 6, 2023
dmitryashutov added a commit that referenced this issue Jan 9, 2023
* Add pyproject.toml

* Remove setup files

* Remove unnecessary tables in the pyproject.toml

* Move tox config to pyproject.toml

* Add build workflow

* Add Pipfile

* Update aidbox launch config

* Skip aiohttp.request mocking tests

Fix once #93 is done

* Remove pytest-mock

* Add mised import to test

* Update build workflow config

* Remove python required version in Pipfile

* Add docker services to build workflow

* Pass env vars for Aidbox container

* Fix typo in the build config

* Fix typoes

* Disable aidbox healthcheck service

* Remove services description

* Move test command to the shell script

* Remove setup python step
@jerkos
Copy link

jerkos commented Apr 13, 2023

Hello,
We are using your library at our company, and we really like it.
I' am interested in contributing for this issue. Are you open for it ?
ClientSession object should be owned by the fhirClient (if so, client should receive args/kwargs to customize its creation) ? Or should it be created outside of the lib ? What do you think ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants