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

Feature request: global patch like in requests-cache? #150

Open
ltalirz opened this issue Mar 24, 2023 · 7 comments
Open

Feature request: global patch like in requests-cache? #150

ltalirz opened this issue Mar 24, 2023 · 7 comments
Labels
enhancement New feature or request

Comments

@ltalirz
Copy link

ltalirz commented Mar 24, 2023

Feature description

Global cache via monkeypatching, just like in requests-cache

import requests
import requests_cache

requests_cache.install_cache('demo_cache')
requests.get('https://httpbin.org/delay/1')

[Apologies if this is not a sensible feature request; I'm new to aiohttp]

Use case

Cache requests of existing Python package that relies on aiohttp.ClientSession (without modifying the package).

Workarounds

Not that I'm aware of. I tried using requests-cache with the code snippet above, but that did not work.

@ltalirz ltalirz added the enhancement New feature or request label Mar 24, 2023
@JWCook
Copy link
Member

JWCook commented Mar 26, 2023

Yes, this is a sensible feature request. I generally consider patching a last resort because of the limitations mentioned here, but when working with another package you don't control, you might not have any other options.

If you let me know what library you're using, I can take a look and see if there are any other options. For example, some libraries use a single internal session object, so you can patch out just that instead of all instances of aiohttp.ClientSession.

@ltalirz
Copy link
Author

ltalirz commented Mar 26, 2023

Hi @JWCook , one example would be https://github.com/openai/openai-python/

See e.g. https://github.com/openai/openai-python/blob/62a208d93ad2e8ffdbd21c4adde5ad9e3b3daea5/openai/api_requestor.py#L694

Independently of whether it would be straightforward to patch an instance of aiohttp.ClientSession in this particular case or not, I believe the global patch feature would still be very useful (with the limitations you mention). Both for running some quick experiments, say, in a Jupyter notebook or for demos where you may want to skip API delay / ensure reproducible results.

@meetps
Copy link

meetps commented Jun 25, 2023

I tried setting the openai.aiosession contextvar with an CachedSession

from aiohttp_client_cache import CachedSession as AIOCachedSession

async with AIOCachedSession(cache=AIOMongoDBBackend('my_async_cache', host="<HOST>", port=27017)) as aiosession:
        openai.aiosession.set(aiosession)
        for i in range(N):
            tasks.append(asyncio.create_task(openai.ChatCompletion.acreate(
            model_name='gpt-3.5-turbo', 
messages=[{"role": "user", "content": "How are you?"}])))
        responses = await asyncio.gather(*tasks)
    return responses

But I am unable to benefit from caching.

@JWCook
Copy link
Member

JWCook commented Jul 12, 2023

@meetps That's strange. I don't have a paid OpenAI account to test that out right now, but from looking over the openapi-python code, your example looks correct.

This example verifies that the internal session made by openapi.api_requestor.aiohttp_session() is a CachedSession.
(And I don't see anywhere else in the library outside of that function that creates a new session object)

import asyncio
import openai
from aiohttp_client_cache import CachedSession, SQLiteBackend

async def test_session():
    async with CachedSession(cache=SQLiteBackend()) as session:
        openai.aiosession.set(session)
        async with openai.api_requestor.aiohttp_session() as internal_session:
            print(internal_session.__class__)

asyncio.run(test_session())

There could be something about the responses that's causing them to not be cached. Could you show me an example response (status code, headers, and content)?

@JWCook
Copy link
Member

JWCook commented Jul 12, 2023

Back to the topic of a global install_cache() feature... Any suggestions for the best way to go about this? In 2023, is there a more sophisticated way to accomplish this than plain old monkey-patching? Here's how requests-cache does it, for reference: requests_cache/patcher.py.

The biggest problem with that is unintentional caching outside of the module in which it's used (the cause of many bug reports!). I'd feel much better about it if, for example, we could patch out aiohttp.ClientSession only for the current module.

@meetps
Copy link

meetps commented Jul 15, 2023

I'm unsure if we'd want to enable direct monkey-patching of asyncio.ClientSession globally. But patching a subset of named sessions using scopes (similar to nest_asyncio 1) and

CACHE_SESSION_MAPPING =  {
  "my_cache_session": <params>
  ...
} 

with nest_asyncio.Session("my_cache_session"):
   # <core components  to be cached>

With this functionality, we can provide users flexibility to install a global patch or annotate their async modules that need to be cached with names and apply a catch only to those sessions. I am not very familiar with the codebase to comment on actual implementation details.

@meetps
Copy link

meetps commented Jul 15, 2023

OpenAI request and response example:

 curl https://api.openai.com/v1/chat/completions \
  -H "Content-Type: application/json" \
  -H "Authorization: Bearer $OPENAI_API_KEY" \
  -d '{
    "model": "gpt-3.5-turbo",
    "messages": [{"role": "system", "content": "You are a helpful assistant."}, {"role": "user", "content": "Hello!"}]
  }'

{
  "id": "chatcmpl-7cerH4sKWODM3PFDOnGzGZcq8ZK4G",
  "object": "chat.completion",
  "created": 1689447879,
  "model": "gpt-3.5-turbo-0613",
  "choices": [
    {
      "index": 0,
      "message": {
        "role": "assistant",
        "content": "Hello! How can I assist you today?"
      },
      "finish_reason": "stop"
    }
  ],
  "usage": {
    "prompt_tokens": 19,
    "completion_tokens": 9,
    "total_tokens": 28
  }
}

Unable to see anything obvious preventing this from being cached.

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

No branches or pull requests

3 participants