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

Simplify MemGPTConfig usage #944

Open
1 of 4 tasks
tombedor opened this issue Jan 30, 2024 · 2 comments
Open
1 of 4 tasks

Simplify MemGPTConfig usage #944

tombedor opened this issue Jan 30, 2024 · 2 comments

Comments

@tombedor
Copy link
Contributor

tombedor commented Jan 30, 2024

See discussion here: https://discord.com/channels/1161736243340640419/1171281929165881384/1201966373308485672

A medium size refactor in which:

  • (optional) remove large commented out blocks of code - they increase maintenance overhead, and can always be retrieved via git
  • writes to the config file are isolated from reads. In other words, decrease the number of "save" calls to "quickstart" and "configure" cli commands
  • the config file is made to be more static, and only instantiated via "MemGPT()"
  • the config is referenced statically, rather than being included in many function arguments
@tombedor
Copy link
Contributor Author

tombedor commented Feb 1, 2024

The final config will look something like this: https://github.com/tombedor/MemGPT/blob/main/memgpt/config.py#L23

with open(yaml_file, "r") as file:
    config_data = yaml.safe_load(file) # not actually a yaml load, keep the existing config format for this change


class MemGPTConfig:
    storage_type: str = config_data["storage_type"]
    storage_uri_env: str = config_data["storage_uri_env"]

    config_path: str = os.path.join(MEMGPT_DIR, "config")
    anon_clientid: str = config_data["anon_clientid"]

    # preset



note that this will still enable overwriting of variables for tests etc

@tombedor
Copy link
Contributor Author

tombedor commented Feb 1, 2024

I'm happy to make these changes and leave it at that, @cpacker / @sarahwooders I think converting to a yaml / rm'ing the Quickstart is another round of changes that could be left alone or tackled later (and that piece is more opinionated, not sure how you feel about that one).

but the changes here will be user-invisible

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

No branches or pull requests

1 participant