letta icon indicating copy to clipboard operation
letta copied to clipboard

bug: default settings value always overides postgres configuration when using server

Open madgrizzle opened this issue 9 months ago • 3 comments

https://github.com/cpacker/MemGPT/blob/7e5cf3e17093bc516f7947fba3dc4fcd0172a403/memgpt/server/server.py#L218

        for memory_type in ("archival", "recall", "metadata"):
            if settings.memgpt_pg_uri:
                # override with env
                setattr(self.config, f"{memory_type}_storage_uri", settings.memgpt_pg_uri)
        self.config.save()

I don't understand why this routine is here because it appears to just override any configuration settings entered in by memgpt configure with what's in settings but it never gets initialized with anything other than hardcoded default memgpt_pg_uri of: postgresql+pg8000://memgpt:memgpt@localhost:5432/memgpt.

You can test this by using postgres docker and in set the configuration set the connection string as follows: ? Enter postgres connection string (e.g. postgresql+pg8000://{user}:{password}@{ip}:5432/{database}): postgresql+pg8000://memgpt:memgpt@localhost:8888/memgpt

To verify its written, check the config file: cat ~/.memgpt/config

[archival_storage] type = postgres path = /home/john/.memgpt/chroma uri = postgresql+pg8000://memgpt:memgpt@localhost:8888/memgpt

Run the server with memgpt server and connection will be refused by the server because the connection string will be overwritten with a port of 5432, and then the config file gets saved!

Run: cat ~/.memgpt/config and you'll see the 5432 has been written in place of 8888.

[archival_storage]
type = postgres
path = /home/john/.memgpt/chroma
uri = postgresql+pg8000://memgpt:memgpt@localhost:5432/memgpt

I tracked it down to the referenced routine, it just seems maybe it's something unfinished?

madgrizzle avatar May 10 '24 00:05 madgrizzle

had the same problem, setting these env variable worked for me

MEMGPT_PG_DB MEMGPT_PG_USER MEMGPT_PG_PASSWORD MEMGPT_PG_URL MEMGPT_PG_HOST

Sapessii avatar May 10 '24 07:05 Sapessii

Ok, thanks, that helps a lot. I'll have to figure out where those get imported in.. I think I'll work on a change to only do the substitution if those values are set.

madgrizzle avatar May 10 '24 10:05 madgrizzle

uggh.. my solution is a bit more hacky than what it already is. Since tests may rely upon a default being returned by setting.memgpt_pg_uri and I didn't see an easy way to make it NOT return the default, then I added a new property called setting.memgpt_pg_uri_no_default and it only returns a uri if the environment variables are set. Otherwise, it returns None and the config file uri continues to be used and not overwritten.

madgrizzle avatar May 10 '24 23:05 madgrizzle