-
Notifications
You must be signed in to change notification settings - Fork 917
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
fix: Exclude unset fields from agent's default settings #628
Conversation
There was a problem hiding this 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 9e9cbcd in 8 seconds
More details
- Looked at
13
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. agents-api/agents_api/models/agent/create_or_update_agent.py:86
- Draft comment:
The use ofmodel_dump(exclude_unset=True)
ensures only explicitly set fields are included, preventing unset fields from overwriting existing settings. This aligns with the PR's intent to exclude unset fields. - Reason this comment was not posted:
Confidence changes required:0%
The change in the PR modifies howdefault_settings
is extracted fromdata
. The new method usesmodel_dump(exclude_unset=True)
, which ensures that only fields that have been explicitly set are included. This is a good change as it prevents default settings from being overwritten with unset fields, aligning with the PR's intent.
Workflow ID: wflow_1EGEAHxCj7VneZl0
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
|
There was a problem hiding this 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! Incremental review on 9874cbc in 10 seconds
More details
- Looked at
17
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. agents-api/agents_api/models/agent/create_or_update_agent.py:87
- Draft comment:
The PR description mentions usingmodel_dump(exclude_unset=True)
, but the code usesmodel_dump(exclude_none=True)
. Please ensure the description matches the code changes. - Reason this comment was not posted:
Comment did not seem useful.
Workflow ID: wflow_faQcNDzLyGoQ7e5q
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
There was a problem hiding this 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! Incremental review on b49f435 in 8 seconds
More details
- Looked at
21
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. agents-api/tests/test_docs_queries.py:44
- Draft comment:
Consider adding a more detailed reason or a reference to a task/issue tracker in the@skip
decorator for better context. - Reason this comment was not posted:
Confidence changes required:33%
The use of@skip
decorator is appropriate for tests that are not ready to be executed. However, it would be beneficial to include a more detailed reason or a reference to a task or issue tracker for better context.
Workflow ID: wflow_KtLWpeQHMIkWoPWk
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Important
create_or_update_agent
now excludes unset fields fromdefault_settings
, and a test is skipped intest_docs_queries.py
.create_or_update_agent
,default_settings
now usesmodel_dump(exclude_unset=True)
to exclude unset fields.model: get docs
intest_docs_queries.py
with a note to execute embedding workflow to fix it.This description was created by for b49f435. It will automatically update as commits are pushed.