Add sorting to task api #1018
Merged
Add sorting to task api #1018
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
<!-- ELLIPSIS_HIDDEN --> > [!IMPORTANT] > Adds sorting functionality to task API with new parameters for sorting direction and column in `client.py` and `agent_protocol.py`. > > - **Behavior**: > - Adds sorting to `get_tasks()` in `client.py` with `order_by_column` and `order` parameters. > - Updates `get_agent_tasks()` in `agent_protocol.py` to accept `sort` and `order` query parameters. > - **Schemas**: > - Adds `OrderBy` and `SortDirection` enums to `tasks.py` for sorting options. > - **Imports**: > - Adds `OrderBy` and `SortDirection` imports in `client.py` and `agent_protocol.py`. > > <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=Skyvern-AI%2Fskyvern-cloud&utm_source=github&utm_medium=referral)<sup> for 3cde532c20c2c11969c48a527610f75a81f595e8. It will automatically update as commits are pushed.</sup> <!-- ELLIPSIS_HIDDEN -->
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 16010bc in 37 seconds
More details
- Looked at
113lines of code in3files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. skyvern/forge/sdk/routes/agent_protocol.py:396
- Draft comment:
Consider adding validation forsortto ensure it is a valid attribute ofTaskModel. This will prevent runtime errors if an invalid column name is passed. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is relevant to the changes made in the diff, as it addresses the newly added 'sort' parameter. Adding validation for the 'sort' parameter could prevent runtime errors if an invalid column name is passed, which is a valid concern. The comment is actionable and suggests a specific improvement to the code quality.
The comment assumes that the 'sort' parameter could be invalid without evidence from the code. It might be unnecessary if the 'OrderBy' type already ensures valid values.
The 'OrderBy' type might not cover all potential invalid inputs, especially if it can be extended or modified elsewhere. Adding explicit validation could be a safeguard.
The comment is relevant and suggests a specific improvement related to the changes in the diff. It should be kept as it addresses a potential issue with the new 'sort' parameter.
Workflow ID: wflow_JRqkSBu1UeMotOwP
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.
| @@ -484,7 +488,12 @@ async def get_tasks( | |||
| query = query.filter(TaskModel.workflow_run_id == workflow_run_id) | |||
| if only_standalone_tasks: | |||
| query = query.filter(TaskModel.workflow_run_id.is_(None)) | |||
| query = query.order_by(TaskModel.created_at.desc()).limit(page_size).offset(db_page * page_size) | |||
| order_by_col = getattr(TaskModel, order_by_column) | |||
There was a problem hiding this comment.
Consider adding validation for order_by_column to ensure it is a valid attribute of TaskModel. This will prevent runtime errors if an invalid column name is passed.
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 16010bc in 37 seconds
More details
- Looked at
113lines of code in3files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. skyvern/forge/sdk/routes/agent_protocol.py:421
- Draft comment:
Ensure thatsortandorderparameters are validated to be within the expected enum values to prevent unexpected behavior. - Reason this comment was not posted:
Confidence changes required:50%
Theget_agent_tasksfunction inagent_protocol.pycorrectly uses the new sorting parameterssortandorder. However, it should ensure that these parameters are validated to be within the expected enum values to prevent unexpected behavior.
Workflow ID: wflow_THKwLjYNFrXpdaIa
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.
| @@ -484,7 +488,12 @@ async def get_tasks( | |||
| query = query.filter(TaskModel.workflow_run_id == workflow_run_id) | |||
| if only_standalone_tasks: | |||
| query = query.filter(TaskModel.workflow_run_id.is_(None)) | |||
| query = query.order_by(TaskModel.created_at.desc()).limit(page_size).offset(db_page * page_size) | |||
| order_by_col = getattr(TaskModel, order_by_column) | |||
There was a problem hiding this comment.
Consider validating order_by_column before using getattr to ensure it is a valid attribute of TaskModel. This can prevent potential runtime errors.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Important
Adds sorting functionality to task API with new parameters for sorting direction and column in
client.pyandagent_protocol.py.get_tasks()inclient.pywithorder_by_columnandorderparameters.get_agent_tasks()inagent_protocol.pyto acceptsortandorderquery parameters.OrderByandSortDirectionenums totasks.pyfor sorting options.OrderByandSortDirectionimports inclient.pyandagent_protocol.py.This description was created by
for 16010bc. It will automatically update as commits are pushed.