Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(agents-api): fix create_agent and create_or_update_agent query #503

Merged
merged 1 commit into from
Sep 18, 2024

Conversation

creatorrr
Copy link
Contributor

@creatorrr creatorrr commented Sep 18, 2024

🚀 This description was created by Ellipsis for commit 382cf1a

fix(agents-api): correct query logic and error handling in agent/session creation

Summary:

Fixes query logic and error handling in agent and session creation, with minor code style adjustments and added TODOs for model validation.

Key points:

  • Behavior:
    • Fixes query logic in create_or_update_agent.py to prevent overwriting settings.
    • Adds AssertionError handling in create_or_update_session.py and create_session.py.
    • Changes return type in create_or_update_session.py to ResourceUpdatedResponse.
  • Code Style:
    • Minor code style adjustments in raise_complete_async.py and transition_step.py.
    • Adds LITELLM_SALT_KEY to .env.example.
  • Misc:
    • Adds TODO comments for model name validation in create_agent.py and create_or_update_agent.py.
    • Fixes a typo in task_execution/__init__.py.

Generated with ❤️ by ellipsis.dev

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.

❌ Changes requested. Reviewed everything up to 382cf1a in 27 seconds

More details
  • Looked at 232 lines of code in 11 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. .env.example:9
  • Draft comment:
    Consider documenting the purpose and usage of LITELLM_SALT_KEY to ensure clarity for developers.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The PR introduces a new environment variable LITELLM_SALT_KEY in the .env.example file. This is a good practice for managing configuration, but it should be documented or explained in the PR description or comments to ensure developers know its purpose and usage.
2. agents-api/agents_api/models/session/create_or_update_session.py:29
  • Draft comment:
    Ensure all assertions provide meaningful error messages to the client, as AssertionError is now mapped to a 400 HTTP status code.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    In create_or_update_session.py, the create_or_update_session function now handles AssertionError by mapping it to a 400 HTTP status code. This is a good practice for handling exceptions, but it should be ensured that all assertions are meaningful and provide clear error messages to the client.
3. agents-api/agents_api/models/session/create_or_update_session.py:32
  • Draft comment:
    Consider providing a more user-friendly error message for the ValueError when both agent and agents are provided.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    In create_or_update_session.py, the function create_or_update_session has a check to ensure only one of agent or agents is provided. This is a good validation step, but the error message could be more user-friendly.
4. agents-api/agents_api/models/session/create_session.py:30
  • Draft comment:
    Ensure all assertions provide meaningful error messages to the client, as AssertionError is now mapped to a 400 HTTP status code.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    In create_session.py, the function create_session now handles AssertionError by mapping it to a 400 HTTP status code. This is a good practice for handling exceptions, but it should be ensured that all assertions are meaningful and provide clear error messages to the client.

Workflow ID: wflow_SJ5k9cy9iI43YrUv


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Copy link
Contributor

@HamadaSalhab HamadaSalhab left a comment

Choose a reason for hiding this comment

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

LGTM

@HamadaSalhab HamadaSalhab merged commit 509f142 into dev Sep 18, 2024
4 of 7 checks passed
@HamadaSalhab HamadaSalhab deleted the x/fix-misc branch September 18, 2024 07:40
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants