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

feat: allow to override config from project #399

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jrmi
Copy link
Contributor

@jrmi jrmi commented Jan 12, 2025

This MR allows to override some of the the global config keys with the project file. In particular we can override the environment variables. The project config and the main config are now merged in one object.

It also refactor the way in which the workspace is detected/constructed so that there is a central point where the logic is located.


Important

This PR enables overriding global configuration with project-specific settings, refactors workspace management, and adds tests for these changes.

  • Configuration Override:
    • Allows overriding global config keys with project-specific settings in config.py.
    • Merges project config and main config into a single object.
    • Introduces ProjectConfig class for project-level settings.
  • Workspace Management:
    • Refactors workspace detection and construction logic in config.py.
    • Uses get_workspace_dir() to determine workspace directory.
  • Environment Variables:
    • Overrides environment variables using project config in config.py.
    • Uses SESSION_NAME and WORKSPACE environment variables for session management.
  • Code Refactoring:
    • Removes logdir and workspace parameters from chat() in chat.py.
    • Refactors session name and workspace handling in cli.py.
  • Testing:
    • Adds tests for global and project-specific configurations in test_config.py.
    • Tests RAG tool message enhancement with and without RAG in test_tools_rag.py.

This description was created by Ellipsis for 60f633a. It will automatically update as commits are pushed.

@jrmi jrmi marked this pull request as draft January 12, 2025 15:51
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Looks good to me! Reviewed everything up to 60f633a in 1 minute and 14 seconds

More details
  • Looked at 891 lines of code in 10 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 drafted comments based on config settings.
1. gptme/config.py:67
  • Draft comment:
    Consider handling the case where SESSION_NAME is not set before calling get_log_dir() to avoid assertion errors. Provide a more informative error message if SESSION_NAME is missing.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
2. gptme/chat.py:87
  • Draft comment:
    Ensure SESSION_NAME is set before calling config.get_workspace_dir() to prevent assertion errors. Consider adding a check and providing a user-friendly error message if SESSION_NAME is missing.
  • Reason this comment was not posted:
    Marked as duplicate.
3. gptme/cli.py:265
  • Draft comment:
    Ensure SESSION_NAME is set before calling config.get_workspace_dir() to prevent assertion errors. Consider adding a check and providing a user-friendly error message if SESSION_NAME is missing.
  • Reason this comment was not posted:
    Marked as duplicate.
4. gptme/eval/agents.py:43
  • Draft comment:
    Ensure SESSION_NAME is set before calling config.get_workspace_dir() to prevent assertion errors. Consider adding a check and providing a user-friendly error message if SESSION_NAME is missing.
  • Reason this comment was not posted:
    Marked as duplicate.
5. gptme/tools/subagent.py:50
  • Draft comment:
    Ensure SESSION_NAME is set before calling config.get_workspace_dir() to prevent assertion errors. Consider adding a check and providing a user-friendly error message if SESSION_NAME is missing.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_4ahcO1TCw7NYFnRA


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@jrmi jrmi force-pushed the feat-pass-down-configuration-to-subprocess branch from af8f5c5 to 4542902 Compare January 12, 2025 20:26
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.

1 participant