Unable to reuse the same Aiogoogle object throughout the app.

Question

Unable to reuse the same Aiogoogle object throughout the app.

Closed this issue 3 years ago · 14 comments

You can't really use one Aiogoogle object in true async fasion due to explicitly setting active_session to None on __aexit__.

Let's assume we have a big application with million of users accessing it every day.
There would be at least one moment in the day when two users want to get something from Google.
Like that:

async def coroutine_with_delay(aiogoogle: Aiogoogle):
    async with aiogoogle as client:
        print("with_delay started!")
        await client.discover("sheets", "v4")
        await asyncio.sleep(5)
        print("with_delay done!")


async def coroutine_no_delay(aiogoogle: Aiogoogle):
    async with aiogoogle as client:
        print("no_delay started!")
        await client.discover("sheets", "v4")
        print("no_delay done!")


async def main():
    aiogoogle = Aiogoogle(service_account_creds=...)

    with_delay, no_delay = await asyncio.gather(
        coroutine_with_delay(aiogoogle),
        coroutine_no_delay(aiogoogle),
    )

The example above fails due to coroutine_no_delay exits the context faster then coroutine_with_delay, so when coroutine_with_delay wants to exit from the context it can't access already closed session as it is None.

Traceback:

with_delay started!
no_delay started!
no_delay done!
with_delay done!
Traceback (most recent call last):
  File "/home/shmoo/projects/kormipravilno/google/test1.py", line 51, in <module>
    asyncio.run(main())
  File "/usr/lib/python3.10/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/usr/lib/python3.10/asyncio/base_events.py", line 641, in run_until_complete
    return future.result()
  File "/home/shmoo/project/test1.py", line 45, in main
    with_delay, no_delay = await asyncio.gather(
  File "/home/shmoo/project/test1.py", line 25, in coroutine_with_delay
    async with aiogoogle as client:
  File "/home/shmoo/.cache/pypoetry/virtualenvs/project-LXfcEK8W-py3.10/lib/python3.10/site-packages/aiogoogle/client.py", line 395, in __aexit__
    await self.active_session.__aexit__(*args)
AttributeError: 'NoneType' object has no attribute '__aexit__'. Did you mean: '__init__'?

Moreover, if you actually did something time expensive with aiogoogle client in coroutine_with_delay instead of just sleeping, the coroutine would just hang. No exceptions until you interrupt the script.

Currently, the only way to go about this is something like:

class Engine:
    """
    Proxy for interaction with Aiogoogle.
    """

    def __init__(
        self,
        *,
        api_key: Optional[str] = None,
        user_creds: Optional[UserCreds] = None,
        client_creds: Optional[ClientCreds] = None,
        service_account_creds: Optional[ServiceAccountCreds] = None,
    ) -> None:
        self.api_key = api_key
        self.user_creds = user_creds
        self.client_creds = client_creds
        self.service_account_creds = service_account_creds

    @property
    def aiogoogle(self) -> Aiogoogle:
        """
        Generates new Aiogoogle object with stored creds.
        """
        return Aiogoogle(
            api_key=self.api_key,
            user_creds=self.user_creds,
            client_creds=self.client_creds,
            service_account_creds=self.service_account_creds,
        )

But that's not reusing, more like reconstructing.

Answer 1 · 2022-04-14T18:28:19.000Z

Moreover, if you actually did something time expensive with aiogoogle client in coroutine_with_delay instead of just sleeping, the coroutine would just hang. No exceptions until you interrupt the script.

I don't really understand why it hangs, as send method should create new session, but that happens.

async def coroutine_with_delay(aiogoogle: Aiogoogle):
    async with aiogoogle as client:
        print("with_delay started!")
        await client.discover("sheets", "v4")
        await asyncio.sleep(5)
        await client.discover("sheets", "v4")
        print("with_delay done!")

Also, the traceback is really weird:

with_delay started!
no_delay started!
no_delay done!
^Cunhandled exception during asyncio.run() shutdown
task: <Task finished name='Task-1' coro=<main() done, defined at /home/shmoo/projects/kormipravilno/google/test1.py:40> exception=AttributeError("'NoneType' object has no attribute '__aexit__'")>
Traceback (most recent call last):
  File "/usr/lib/python3.10/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/usr/lib/python3.10/asyncio/base_events.py", line 628, in run_until_complete
    self.run_forever()
  File "/usr/lib/python3.10/asyncio/base_events.py", line 595, in run_forever
    self._run_once()
  File "/usr/lib/python3.10/asyncio/base_events.py", line 1845, in _run_once
    event_list = self._selector.select(timeout)
  File "/usr/lib/python3.10/selectors.py", line 469, in select
    fd_event_list = self._selector.poll(timeout, max_ev)
KeyboardInterrupt

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/shmoo/projects/kormipravilno/google/test1.py", line 46, in main
    with_delay, no_delay = await asyncio.gather(
  File "/home/shmoo/projects/kormipravilno/google/test1.py", line 25, in coroutine_with_delay
    async with aiogoogle as client:
  File "/home/shmoo/.cache/pypoetry/virtualenvs/kp-google-LXfcEK8W-py3.10/lib/python3.10/site-packages/aiogoogle/client.py", line 395, in __aexit__
    await self.active_session.__aexit__(*args)
AttributeError: 'NoneType' object has no attribute '__aexit__'
unhandled exception during asyncio.run() shutdown
task: <Task finished name='Task-2' coro=<coroutine_with_delay() done, defined at /home/shmoo/projects/kormipravilno/google/test1.py:24> exception=AttributeError("'NoneType' object has no attribute '__aexit__'")>
Traceback (most recent call last):
  File "/usr/lib/python3.10/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/usr/lib/python3.10/asyncio/base_events.py", line 628, in run_until_complete
    self.run_forever()
  File "/usr/lib/python3.10/asyncio/base_events.py", line 595, in run_forever
    self._run_once()
  File "/usr/lib/python3.10/asyncio/base_events.py", line 1845, in _run_once
    event_list = self._selector.select(timeout)
  File "/usr/lib/python3.10/selectors.py", line 469, in select
    fd_event_list = self._selector.poll(timeout, max_ev)
KeyboardInterrupt

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/shmoo/projects/kormipravilno/google/test1.py", line 46, in main
    with_delay, no_delay = await asyncio.gather(
  File "/home/shmoo/projects/kormipravilno/google/test1.py", line 25, in coroutine_with_delay
    async with aiogoogle as client:
  File "/home/shmoo/.cache/pypoetry/virtualenvs/kp-google-LXfcEK8W-py3.10/lib/python3.10/site-packages/aiogoogle/client.py", line 395, in __aexit__
    await self.active_session.__aexit__(*args)
AttributeError: 'NoneType' object has no attribute '__aexit__'
Traceback (most recent call last):
  File "/home/shmoo/projects/kormipravilno/google/test1.py", line 52, in <module>
    asyncio.run(main())
  File "/usr/lib/python3.10/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/usr/lib/python3.10/asyncio/base_events.py", line 628, in run_until_complete
    self.run_forever()
  File "/usr/lib/python3.10/asyncio/base_events.py", line 595, in run_forever
    self._run_once()
  File "/usr/lib/python3.10/asyncio/base_events.py", line 1845, in _run_once
    event_list = self._selector.select(timeout)
  File "/usr/lib/python3.10/selectors.py", line 469, in select
    fd_event_list = self._selector.poll(timeout, max_ev)
KeyboardInterrupt

Answer 2 · 2022-04-14T19:25:09.000Z

Hey, thanks for raising the issue.

Why not use different session object for each user?

Also, when you use the same object more than once you'll run into a limitation this limitation of AioHTTP where when you aenter it once and aexit you cannot re aenter the same object again.

aenter here opens the Aiohttp session and aexit closes it. You cannot reopen a closed session. And I'm not sure if using the same session for the entire life of a server is a good idea in the first place. What if it got interrupted for some random reason? Your app will still attempt to use this same session object, which will make your app fail.

Here's how I solved it once before

from aiogoogle import Aiogoogle as LibAiogoogle

Aiogoogle = lambda: Aiogoogle(**your_options)
api = None

async def init():
  global api
  async with Aiogoogle() as google:
    api = await google.discover(...)

async def task_1():
  async with Aiogoogle() as google:
    result_1 = await api.resource.method(...)
 
async def task_2():
  async with Aiogoogle() as google:
    result_1 = await api.resource.method(...)

async def main():
  await init()
  asyncio.gather(task1, task2)

Answer 3 · 2022-04-14T19:52:51.000Z

Thanks for the explanation!

I think you misunderstood me though.
For example, my app uses only app's service account. From developer's that's using aiogoogle perspective, I think it would be nice to have a single Aiogoogle object with my ServiceAccountCreds, as the developer doesn't need to know about the internals of Aiogoogle: session creating e.g.

I think the main issue here is not

explicitly setting active_session to None on __aexit__

, but Aiogoogle not guarantying that active_session is actually active throughout the context manager.

In other words, a single AioHTTP session doesn't need to be used multiple times, but it has to be guaranteed to be usable and disposable even with this example.

Answer 4 · 2022-04-14T19:54:52.000Z

Maybe, something like Session pool would work?

Answer 5 · 2022-04-14T20:18:29.000Z

Active session is guaranteed to be active when you haven't exited the session manager yet. We can expose a setup function that accepts the arguments of Aiogoogle and provides it to the Aiogoogle class via a closure. e.g. ``` From aiogoogle import setup aiogoogle_factory = setup(service_account="..") async def main(): async with aiogoogle_factory() as google: task_1() async with aiogoogle_factory() as google: task_2() ``` aiogoogle_factory creates instantiates a new Aiogoogle object everytime it's called using the args provided in setup. Does that make sense?

…

On Thu, Apr 14, 2022, 9:55 PM Shmookoff ***@***.***> wrote: Maybe, something like Session pool would work? — Reply to this email directly, view it on GitHub <#91 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AGPFI5L32Q3PBMOGQC5AQL3VFBZZPANCNFSM5TONYUOQ> . You are receiving this because you commented.Message ID: ***@***.***>

Answer 6 · 2022-04-14T21:54:42.000Z

I understand what you're trying to say. But creating whole new instance may seem somewhat inconvenient. There is a PR hopefully explaining what I'm trying to achieve.

Answer 7 · 2022-04-14T23:11:23.000Z

Pretty cool solution! Can you fix the lint errors? and I'll merge it and create a new release. Thanks!! @Shmookoff

Answer 8 · 2022-04-15T10:50:59.000Z

Done! Also, I removed sessions property, as the same can be achieved storing the session itself in context variable. @omarryhan

Answer 9 · 2022-04-15T11:24:28.000Z

The only caveat with my approach is as follows:
If you opened nested context within a context with the same Aiogoogle object, nested context would use the same session as the main context.
At the end of the nested context, __aexit__ would close the shared session.
If after the nested context, you use some method that calls _ensure_session_set, new session would be created and at the end of main context that new session would be closed.

async def coroutine_with_delay(aiogoogle: Aiogoogle):
    async with aiogoogle as client:
        print("with_delay started!")

        async with aiogoogle as client1:
            print("nested started!")
            await client1.discover("sheets", "v4")
            print("nested done!")

        await client.discover("sheets", "v4")  # <--- Here
        print("with_delay done!")

opening session = <aiogoogle.sessions.aiohttp_session.AiohttpSession object at 0x7faf1da38bb0>
with_delay started!
opening session = <aiogoogle.sessions.aiohttp_session.AiohttpSession object at 0x7faf1da38bb0>
nested started!
nested done!
closing session = <aiogoogle.sessions.aiohttp_session.AiohttpSession object at 0x7faf1da38bb0>
with_delay done!
closing session = <aiogoogle.sessions.aiohttp_session.AiohttpSession object at 0x7faf1da3bdf0>

However, if you don't

use some method that calls _ensure_session_set

, the main context would try to close non-existent session, and fail.

async def coroutine_with_delay(aiogoogle: Aiogoogle):
    async with aiogoogle as client:
        print("with_delay started!")
        async with aiogoogle as client1:
            print("nested started!")
            await client1.discover("sheets", "v4")
            print("nested done!")

        print("with_delay done!")

opening session = <aiogoogle.sessions.aiohttp_session.AiohttpSession object at 0x7f8a6b134a30>
with_delay started!
opening session = <aiogoogle.sessions.aiohttp_session.AiohttpSession object at 0x7f8a6b134a30>
nested started!
nested done!
closing session = <aiogoogle.sessions.aiohttp_session.AiohttpSession object at 0x7f8a6b134a30>
with_delay done!
closing session = None
Traceback (most recent call last):
  File "/home/shmoo/projects/kormipravilno/google/test1.py", line 54, in <module>
    asyncio.run(main())
  File "/usr/lib/python3.10/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/usr/lib/python3.10/asyncio/base_events.py", line 641, in run_until_complete
    return future.result()
  File "/home/shmoo/projects/kormipravilno/google/test1.py", line 48, in main
    with_delay = await asyncio.gather(
  File "/home/shmoo/projects/kormipravilno/google/test1.py", line 25, in coroutine_with_delay
    async with aiogoogle as client:
  File "/home/shmoo/.cache/pypoetry/virtualenvs/kp-google-LXfcEK8W-py3.10/lib/python3.10/site-packages/aiogoogle/client.py", line 407, in __aexit__
    await session.__aexit__(*args)
AttributeError: 'NoneType' object has no attribute '__aexit__'. Did you mean: '__init__'?

I think this is a rare use-case, and I can't really imagine when one might want to do that.

Answer 10 · 2022-04-15T11:31:58.000Z

We might want to raise an exception stating that you cannot open nested contexts using the same Aiogoogle object if this occurs.

Answer 11 · 2022-04-15T11:43:34.000Z

Or we can just _ensure_session_set on __aexit__.

Answer 12 · 2022-04-15T12:26:29.000Z

Makes sense. I think an exception would be better 🤔 Can't imagine a scenario where one would want to open a nested context. What do you think?

Answer 13 · 2022-04-15T14:16:56.000Z

Yeap, an exception would be better. Updated the PR.

Answer 14 · 2022-04-15T14:36:37.000Z

Thank you so much!
Released in v.4.0.0
Bumped it up a major version, because the .active_session was a public var, and I remember someone showed me a snipped before were they were actively managing it.