-
Notifications
You must be signed in to change notification settings - Fork 890
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(agents-api): Return full objects instead of ResourceCreated/Updated #1089
Conversation
Signed-off-by: Diwank Singh Tomer <[email protected]>
CI Feedback 🧐(Feedback updated until commit bf84810)A test triggered by this PR failed. Here is an AI-generated analysis of the failure:
|
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
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 ac1d6b2 in 1 minute and 2 seconds
More details
- Looked at
3668
lines of code in76
files - Skipped
1
files when reviewing. - Skipped posting
5
drafted comments based on config settings.
1. typespec/users/endpoints.tsp:14
- Draft comment:
TheCreateEndpoint
,CreateOrUpdateEndpoint
,UpdateEndpoint
, andPatchEndpoint
interfaces now include aType
parameter to specify the response type. This change ensures that the response type is explicitly defined asUser
, aligning with the PR's intent to return full objects instead ofResourceCreated/Updated
responses. - Reason this comment was not posted:
Comment did not seem useful.
2. typespec/common/interfaces.tsp:36
- Draft comment:
TheCreateEndpoint
,CreateOrUpdateEndpoint
,UpdateEndpoint
, andPatchEndpoint
interfaces now include aType
parameter to specify the response type. This change ensures that the response type is explicitly defined, aligning with the PR's intent to return full objects instead ofResourceCreated/Updated
responses. - Reason this comment was not posted:
Marked as duplicate.
3. typespec/common/interfaces.tsp:52
- Draft comment:
TheCreateOrUpdateEndpoint
interface now includes aType
parameter to specify the response type. This change ensures that the response type is explicitly defined, aligning with the PR's intent to return full objects instead ofResourceCreated/Updated
responses. - Reason this comment was not posted:
Marked as duplicate.
4. typespec/common/interfaces.tsp:74
- Draft comment:
TheUpdateEndpoint
interface now includes aType
parameter to specify the response type. This change ensures that the response type is explicitly defined, aligning with the PR's intent to return full objects instead ofResourceCreated/Updated
responses. - Reason this comment was not posted:
Marked as duplicate.
5. typespec/common/interfaces.tsp:96
- Draft comment:
ThePatchEndpoint
interface now includes aType
parameter to specify the response type. This change ensures that the response type is explicitly defined, aligning with the PR's intent to return full objects instead ofResourceCreated/Updated
responses. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_Hm17ya5BEHDuwPXY
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
be1e58c
to
c5dba13
Compare
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 c5dba13 in 24 seconds
More details
- Looked at
34
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
2
drafted comments based on config settings.
1. agents-api/agents_api/queries/entries/create_entries.py:8
- Draft comment:
Reorder imports to follow the convention: standard library imports, third-party imports, and then local application imports. - Reason this comment was not posted:
Confidence changes required:50%
The import order in Python should follow the convention of standard library imports, third-party imports, and then local application imports. The current order increate_entries.py
does not follow this convention.
2. agents-api/tests/test_task_queries.py:40
- Draft comment:
Add an assertion to verify that the task was created successfully. This ensures the test checks the expected outcome. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_KKVBXkbTn4j2IxbY
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Co-authored-by: qodo-merge-pro-for-open-source[bot] <189517486+qodo-merge-pro-for-open-source[bot]@users.noreply.github.com>
Signed-off-by: Diwank Singh Tomer <[email protected]>
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 128534c in 13 seconds
More details
- Looked at
14
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
0
drafted comments based on config settings.
Workflow ID: wflow_02vMlhWJuoTMgFTQ
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 0861a1b in 31 seconds
More details
- Looked at
26
lines of code in2
files - Skipped
0
files when reviewing. - Skipped posting
1
drafted comments based on config settings.
1. agents-api/agents_api/queries/docs/create_doc.py:59
- Draft comment:
The@pg_query()
decorator should be used without parentheses.
@pg_query
- Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_VZAbvPgxCvomY2zn
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
""" | ||
|
||
|
||
class Main(BaseModel): |
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.
There's something wrong with this file in general. This is the map
step. And seems like the old Main
model is called MainModel
now.
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.
i guess it's typespec issue ?
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.
❌ Changes requested. Incremental review on 4d3dd14 in 1 minute and 3 seconds
More details
- Looked at
786
lines of code in17
files - Skipped
0
files when reviewing. - Skipped posting
5
drafted comments based on config settings.
1. agents-api/agents_api/queries/docs/utils.py:43
- Draft comment:
Ensure that theembeddings
key is handled properly to avoid potentialKeyError
. Consider usingd.get("embeddings", [])
to provide a default value. - Reason this comment was not posted:
Comment did not seem useful.
2. agents-api/agents_api/queries/entries/create_entries.py:66
- Draft comment:
Thepg_query
decorator hasdebug=True
, which might not be intended for production. Consider removing it or controlling it via a configuration setting. - Reason this comment was not posted:
Comment looked like it was already resolved.
3. agents-api/agents_api/queries/tasks/create_task.py:100
- Draft comment:
Thepg_query
decorator is missing a return type annotation forfetchrow
. Ensure it includesfetchrow
as a possible return type. - Reason this comment was not posted:
Confidence changes required:50%
Thecreate_task
function increate_task.py
is missing a return type annotation for thepg_query
decorator. It should includefetchrow
as a possible return type.
4. agents-api/agents_api/queries/tasks/create_or_update_task.py:133
- Draft comment:
Thepg_query
decorator is missing a return type annotation forfetchrow
. Ensure it includesfetchrow
as a possible return type. - Reason this comment was not posted:
Confidence changes required:50%
Thecreate_or_update_task
function increate_or_update_task.py
is missing a return type annotation for thepg_query
decorator. It should includefetchrow
as a possible return type.
5. agents-api/agents_api/routers/tasks/create_task_execution.py:118
- Draft comment:
Thepg_query
decorator is missing a return type annotation forfetchrow
. Ensure it includesfetchrow
as a possible return type. - Reason this comment was not posted:
Confidence changes required:50%
Thecreate_task_execution
function increate_task_execution.py
is missing a return type annotation for thepg_query
decorator. It should includefetchrow
as a possible return type.
Workflow ID: wflow_yh1xpV3lVQoRBUYv
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.
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 bf84810 in 21 seconds
More details
- Looked at
117
lines of code in6
files - Skipped
0
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. agents-api/agents_api/queries/tasks/create_or_update_task.py:8
- Draft comment:
The import order ofspec_to_task
andtask_to_spec
is inconsistent across files. Consider maintaining a consistent order for better readability. - Reason this comment was not posted:
Confidence changes required:50%
The import statement fortask_to_spec
andspec_to_task
is inconsistent across files. It should be consistent for better readability and maintainability.
2. agents-api/agents_api/queries/tasks/create_task.py:8
- Draft comment:
The import order ofspec_to_task
andtask_to_spec
is inconsistent across files. Consider maintaining a consistent order for better readability. - Reason this comment was not posted:
Confidence changes required:50%
The import statement fortask_to_spec
andspec_to_task
is inconsistent across files. It should be consistent for better readability and maintainability.
3. agents-api/agents_api/queries/tasks/patch_task.py:7
- Draft comment:
The import order ofspec_to_task
andtask_to_spec
is inconsistent across files. Consider maintaining a consistent order for better readability. - Reason this comment was not posted:
Confidence changes required:50%
The import statement fortask_to_spec
andspec_to_task
is inconsistent across files. It should be consistent for better readability and maintainability.
4. agents-api/agents_api/queries/tasks/update_task.py:7
- Draft comment:
The import order ofspec_to_task
andtask_to_spec
is inconsistent across files. Consider maintaining a consistent order for better readability. - Reason this comment was not posted:
Confidence changes required:50%
The import statement fortask_to_spec
andspec_to_task
is inconsistent across files. It should be consistent for better readability and maintainability.
Workflow ID: wflow_8ug4uq5dX7RGDEwm
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
User description
Signed-off-by: Diwank Singh Tomer [email protected]
PR Type
Enhancement, Bug fix, Tests
Description
Replaced
ResourceCreatedResponse
andResourceUpdatedResponse
with full object models across APIs.Added new utility functions and refactored existing ones for better data transformation.
transform_doc
utility for consistent document transformation.Enhanced test coverage and updated existing tests to align with new return types.
Updated TypeSpec definitions and dependencies for improved API documentation and compatibility.
Changes walkthrough 📝
15 files
Added new models for task creation and updates
Refactored integration definitions and updated setup parameters
Changed return type to full document object
Replaced transformation function with centralized utility
Replaced transformation function with centralized utility
Updated patch agent query to return full agent object
Changed return type to dictionary for task execution
Updated file creation to return full file object
Updated task creation to return full task object
Updated patch agent router to return full agent object
Updated document creation to return full document object
Updated task creation or update to return full task object
Updated update agent router to return full agent object
Updated patch task query to return full task object
Updated entry creation to return full entry object
5 files
Updated tests to validate full document objects
Updated task tests to validate full task objects
Adjusted session tests for new return types
Adjusted test fixtures for new document return types
Updated agent tests to validate full agent objects
57 files
Important
Replaced
ResourceCreatedResponse
andResourceUpdatedResponse
with full object models across APIs, updating return types, OpenAPI specs, utilities, and tests.ResourceCreatedResponse
andResourceUpdatedResponse
with full object models in APIs.transform_doc
utility for document transformation.This description was created by for bf84810. It will automatically update as commits are pushed.