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

Issue-111 #122

Merged
merged 5 commits into from
Jan 5, 2024
Merged

Issue-111 #122

merged 5 commits into from
Jan 5, 2024

Conversation

bryce-seefieldt
Copy link
Contributor

@bryce-seefieldt bryce-seefieldt commented Dec 13, 2023

Issue-111 - Partial implementation of config_export()

Proposed changes

Partial solution provided for Issue-111

  • export-config() function defined in config.py. Currently this function is not called within the program, but was tested and is functional if called as the last function in main.main().

  • Further discussion and clarification is required to address number 2 in checklist below. I need some guidance on where in the program I can query to find the current environment, persona, UI settings. and model type.

  • Also would be helpful to understand how/where/when the export_config() function would be called to better understand the current state of the program at the time of executing the function.

  • 1. Define a function export_config() to handle the export process.

  • 2. Capture NeoGPT-related details such as version, environment, persona, UI settings, model type, and export date.

  • 3. Include model-specific information like model name, file, and embedding model.

  • 4. Capture database details, including the parent directory.

  • 5. Include directory information, such as the source directory and workspace directory.

  • 6. Capture memory configurations, including the default memory key.

  • 7. Save the configuration to a YAML file, e.g., "settings/settings.yaml".

Further discussion and clarification is required to

Checks

  • All commits in this Pull Request are signed and Verified by Github.

Issue-111 Partial implementation of functionality requested.
@neokd
Copy link
Owner

neokd commented Dec 14, 2023

The function will be called in the CLI with --export xyz.yaml to export the current configuration to the file.

@neokd
Copy link
Owner

neokd commented Dec 14, 2023

For number 2 we can just add the export date and time for now. Later on we can integrate other things in it.

@neokd
Copy link
Owner

neokd commented Dec 16, 2023

@bryce-seefieldt any updates?

@bryce-seefieldt
Copy link
Contributor Author

bryce-seefieldt commented Jan 1, 2024

@neokd I think this Pull Request covers the creation of the export_config function. Please let me know if you have had a chance to review the code for the function and have any additions or changes you'd like to see implemented.

I have added the --export arg parse argument that would receive the export command line switch and run the export_config function. It saves the config settings in neogpt/settings directory. It allows user to provide an optional paramter for the config file name and defaults to settings.yml if no filename is provided.

@bryce-seefieldt bryce-seefieldt marked this pull request as ready for review January 1, 2024 18:32
Allows config.py export_config() to be run from CLI
Exports configuration settings to YAML file
Added fucntionality to define and validate YAML filename for exporting configuration settings.
@neokd
Copy link
Owner

neokd commented Jan 3, 2024

@bryce-seefieldt the code looks good i have just reverted the UI config since there was a leak when using FAISS can you just make those small tweaks and create a new PR. also can you attach an config file?

@neokd
Copy link
Owner

neokd commented Jan 5, 2024

@bryce-seefieldt Thanks for the contribution.

@neokd neokd merged commit 1bba04e into neokd:main Jan 5, 2024
@bryce-seefieldt bryce-seefieldt deleted the issue-111 branch January 11, 2024 19:25
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

Successfully merging this pull request may close these issues.

2 participants