Skip to content

HttpUtils Aggregation Pattern #349

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.

Sign up for GitHub

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

Closed
MikePresman wants to merge 1 commit into from
Closed

HttpUtils Aggregation Pattern #349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
aggregation_2 changes
  • Loading branch information
@MikePresman
MikePresman committed Jul 16, 2021
commit b57d0e96347b521fd0cb43628c56aa34f75aeee8
8 changes: 4 additions & 4 deletions app/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@
from .config import get_settings
from .data import data_source
from .routers import V1, V2
from .utils.httputils import setup_client_session, teardown_client_session
from .utils.httputils import Session

# ############
# FastAPI App
# ############
LOGGER = logging.getLogger("api")

session = Session()
Copy link
Collaborator

@Kilo59 Kilo59 Jul 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point of creating an instance of Session?

If there's only meant to be one Client Session for the lifetime of the application why are we tempting developers to create multiple Session instances by defining a class in the first place?

Seems everything could be a class method or attribute.
But at that point why have a class at all?

Maybe just to act as a container for http functions and attributes so we only have to import one object from the module. That's certainly reasonable.

SETTINGS = get_settings()

if SETTINGS.sentry_dsn: # pragma: no cover
Expand All @@ -37,8 +37,8 @@
version="2.0.4",
docs_url="/",
redoc_url="/docs",
on_startup=[setup_client_session],
on_shutdown=[teardown_client_session],
on_startup=[session.setup_client_session],
on_shutdown=[session.teardown_client_session],
)

# #####################
Expand Down
35 changes: 18 additions & 17 deletions app/utils/httputils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,28 +3,29 @@

from aiohttp import ClientSession

# Singleton aiohttp.ClientSession instance.
CLIENT_SESSION: ClientSession

class Session:
__CLIENT_SESSION: ClientSession
__LOGGER = logging.getLogger(__name__)

async def setup_client_session(self) -> None:
"""Set up the application-global aiohttp.ClientSession instance.

LOGGER = logging.getLogger(__name__)
aiohttp recommends that only one ClientSession exist for the lifetime of an application.
See: https://docs.aiohttp.org/en/stable/client_quickstart.html#make-a-request

"""
self.__LOGGER.info("Setting up global aiohttp.ClientSession.")
self.__CLIENT_SESSION = ClientSession()


async def setup_client_session():
"""Set up the application-global aiohttp.ClientSession instance.

aiohttp recommends that only one ClientSession exist for the lifetime of an application.
See: https://docs.aiohttp.org/en/stable/client_quickstart.html#make-a-request
async def teardown_client_session():
"""Close the application-global aiohttp.ClientSession.
"""
self.__LOGGER.info("Closing global aiohttp.ClientSession.")
await self.__CLIENT_SESSION.close()


"""
global CLIENT_SESSION # pylint: disable=global-statement
LOGGER.info("Setting up global aiohttp.ClientSession.")
CLIENT_SESSION = ClientSession()


async def teardown_client_session():
"""Close the application-global aiohttp.ClientSession.
"""
global CLIENT_SESSION # pylint: disable=global-statement
LOGGER.info("Closing global aiohttp.ClientSession.")
await CLIENT_SESSION.close()