PrivateCoffee / matrix-gptbot

GPT Chatbot for Matrix
Other
8 stars 3 forks source link

Bugfixes #2

Closed justin-russell closed 1 year ago

justin-russell commented 1 year ago

Hey, I love your bot implementation! I've fixed a few issues I've run across when using it.

Migrations

When from_version was None, migrations were off by one since get_version was returning 0, but migration files were starting at 1. Also due to the nature of range, the latest migration was being excluded. Got those fixed!

Blocking OpenAI API Calls

I also found a few blocking pieces when interacting with the OpenAI API. I switched to using their async functions, so now requests can be made simultaneously.

Occasional OpenAI API Failures

After making the OpenAI API calls to async, I have found that I occasionally run into this error from OpenAI:

Error generating response: That model is currently overloaded with other requests. You can retry your request, or contact us through our help center at help.openai.com if the error persists. (Please include the request ID <request_id> in your message.)

I'm assuming they are rate limiting, or their model is just overloaded?

To counteract this, I added some retry logic around OpenAI API requests. For now, I've set it to retry up to 5 times before giving up. The functionality is the same, just whenever the number of retries runs out, exception is raised.

Let me know if you have any questions!

kumitterer commented 1 year ago

Hi! Thanks for those fixes and for cleaning up a little (yeah, the code is a mess, I'm sorry)!

I'm not entirely sure what the issue is you are trying to fix in the migrations part – creating a new database seems to work just fine for me. Could you elaborate on that, please?

When I first created the bot, I actually tried using the async functions provided by the openai package, but those just seemed to hang forever, which is why I resorted to using the regular functions instead. If async function calls are working now, that would be great, of course.

Assuming you are not using the free trial of the API anymore, the failures you are seeing should not be due to rate limits, although I find it a little weird that you are only seeing this issue using the async functions... πŸ€”

Overall, your changes look good to me. I will test them today – if everything works as expected, I will merge the PR later today (although maybe with changes to the migrations part).

justin-russell commented 1 year ago

Hi! Thanks for those fixes and for cleaning up a little (yeah, the code is a mess, I'm sorry)!

Honestly, I like the structure! I really like how things are organized with callbacks, etc. Things always get a little messy once more features get added. πŸ˜„

I'm not entirely sure what the issue is you are trying to fix in the migrations part – creating a new database seems to work just fine for me. Could you elaborate on that, please?

I seemed to run into some issues with applying migrations on a fresh database:

Traceback (most recent call last):
  File "/home/justin/.pyenv/versions/3.10.5/lib/python3.10/runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/home/justin/.pyenv/versions/3.10.5/lib/python3.10/runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "/home/justin/Documents/opensource/matrix-gptbot/gptbot.py", line 33, in <module>
    asyncio.run(bot.run())
  File "/home/justin/.pyenv/versions/3.10.5/lib/python3.10/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "/home/justin/.pyenv/versions/3.10.5/lib/python3.10/asyncio/base_events.py", line 646, in run_until_complete
    return future.result()
  File "/home/justin/Documents/opensource/matrix-gptbot/classes/bot.py", line 591, in run
    before, after = migrate(self.database)
  File "/home/justin/Documents/opensource/matrix-gptbot/migrations/__init__.py", line 50, in migrate
    MIGRATIONS[version + 1](db)
  File "/home/justin/Documents/opensource/matrix-gptbot/migrations/migration_2.py", line 133, in migration
    cursor.execute(
duckdb.CatalogException: Catalog Error: Table with name migrations does not exist!
Did you mean "accounts"?

I assumed it may have been because the first migration was being skipped somehow. Through our discussion above, what you mentioned seems to have worked (checking if version + 1 is in MIGRATIONS).

When I first created the bot, I actually tried using the async functions provided by the openai package, but those just seemed to hang forever, which is why I resorted to using the regular functions instead. If async function calls are working now, that would be great, of course.

That's weird! Well, things seem to be working. I was able to test by opening two matrix rooms in separate tabs, then running the same query (tell me a story about dogs). In the original implementation, whatever query ran first seemed to block the other one (only one of them was typing). Same thing with using !gptbot imagine dog wearing glasses. Things seem to be working great now!

Assuming you are not using the free trial of the API anymore, the failures you are seeing should not be due to rate limits, although I find it a little weird that you are only seeing this issue using the async functions... πŸ€”

Yeah, you're right. I think it could've just been the model being overwhelmed by the volume of requests from everyone. I, and probably you as well, have occasionally run into errors when using the Chat GPT interface where you get some error and have to retry the request. I'm pretty sure it was just the same thing as that.

Overall, your changes look good to me. I will test them today – if everything works as expected, I will merge the PR later today (although maybe with changes to the migrations part).

Awesome! Thanks for building this thing man. It's pretty nice and easy to use! Have a good one! πŸ˜„