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

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

Open
madgrizzle opened this issue May 10, 2024 · 3 comments

Comments

@madgrizzle
Copy link
Contributor

for memory_type in ("archival", "recall", "metadata"):

        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?

@Sapessii
Copy link

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

@madgrizzle
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: To triage
Development

No branches or pull requests

2 participants