Executions queries #988
Executions queries #988
Conversation
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
CI Failure Feedback 🧐(Checks updated until commit ce47442)
✨ CI feedback usage guide:The CI feedback tool (
In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR: where Configuration options
See more information about the |
whiterabbit1983
force-pushed
the
f/executions-queries
branch
from
bfdd1d5 to
b2994bd
Compare
| @@ -45,8 +45,8 @@ | |||
| async def get_execution( | |||
| *, | |||
| execution_id: UUID, | |||
| ) -> tuple[str, dict]: | |||
| ) -> tuple[str, list]: | |||
| return ( | |||
There was a problem hiding this comment.
you can add a third return value "fetchrow" and add rewrap_exception with asyncpg.exceptions.NoDataFoundError, this will return 404 in case the execution doesn't exist.
Make sure to refactor the return type of the function, and you can look at get_agent to see an example.
| ] | ||
|
|
||
| return (queries, {"task_token": task_token, "transition_id": transition_id}) | ||
| return ( |
There was a problem hiding this comment.
same comment, you can add "fetchrow"
| # :assert some | ||
| # """ | ||
|
|
||
| return ( |
There was a problem hiding this comment.
same comment, you can add "fetchrow"
|
|
||
| :limit 1 | ||
| """ | ||
| execution_id = str(execution_id) | ||
|
|
||
| return ( |
There was a problem hiding this comment.
same comment, you can add "fetchrow"
| return ( | ||
| sql_query, |
There was a problem hiding this comment.
before returning you could catch 2 exceptions where limit and offset values are out of bounds (negative number for limit for example)
whiterabbit1983
force-pushed
the
f/executions-queries
branch
from
3232ae9 to
7ed5fda
Compare
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to ce47442 in 1 minute and 47 seconds
More details
- Looked at
2318lines of code in17files - Skipped
0files when reviewing. - Skipped posting
5drafted comments based on config settings.
1. agents-api/agents_api/queries/executions/create_execution.py:94
- Draft comment:
Ensureexecution_datacontains theinputkey to avoid potential KeyError.
execution_data.get("input", {})
- Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The key insight is that data is annotated with dict_like(CreateExecutionRequest), which means it must match the CreateExecutionRequest schema. If 'input' was optional, it would be marked as such in the schema. Since it's being used directly without any optional handling, it's likely a required field in CreateExecutionRequest. The type system would catch missing required fields.
I don't have direct access to the CreateExecutionRequest schema definition, so I'm making assumptions about the required fields. The type checking could be disabled at runtime.
The @beartype decorator enforces runtime type checking, and dict_like validation would fail if required fields were missing. The code is already properly validated.
The comment should be deleted because 'input' is a required field that's validated by both static typing and runtime checks. Using .get() would hide real errors.
2. agents-api/agents_api/queries/executions/create_execution_transition.py:148
- Draft comment:
Ensurestep_definitionandstep_labelare correctly set if needed.
transition_data.get("step_definition", {}),
transition_data.get("step_label", None),
- Reason this comment was not posted:
Comment did not seem useful.
3. agents-api/agents_api/queries/executions/get_execution_transition.py:54
- Draft comment:
Remove unuseddeveloper_idparameter if not needed. - Reason this comment was not posted:
Confidence changes required:50%
Inget_execution_transition.py, thedeveloper_idparameter is not used. This could be a potential oversight or a placeholder for future use. It should be removed if not needed.
4. agents-api/agents_api/queries/executions/get_paused_execution_token.py:38
- Draft comment:
Remove unuseddeveloper_idparameter if not needed. - Reason this comment was not posted:
Confidence changes required:50%
Inget_paused_execution_token.py, thedeveloper_idparameter is not used. This could be a potential oversight or a placeholder for future use. It should be removed if not needed.
5. agents-api/agents_api/queries/executions/lookup_temporal_data.py:31
- Draft comment:
Remove unuseddeveloper_idparameter if not needed. - Reason this comment was not posted:
Confidence changes required:50%
Inlookup_temporal_data.py, thedeveloper_idparameter is not used. This could be a potential oversight or a placeholder for future use. It should be removed if not needed.
Workflow ID: wflow_YaA2hQk6Dvyv1eqf
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
PR Type
Enhancement, Bug fix
Description
Major refactoring of execution queries to migrate from Cozo to PostgreSQL:
Changes walkthrough 📝
13 files
__init__.py
Simplify execution transitions module importsagents-api/agents_api/queries/executions/init.py
count_executions.py
Refactor count executions query for PostgreSQLagents-api/agents_api/queries/executions/count_executions.py
create_execution.py
Migrate execution creation to PostgreSQLagents-api/agents_api/queries/executions/create_execution.py
create_execution_transition.py
Convert execution transition creation to PostgreSQLagents-api/agents_api/queries/executions/create_execution_transition.py
create_temporal_lookup.py
Migrate temporal lookup creation to PostgreSQLagents-api/agents_api/queries/executions/create_temporal_lookup.py
get_execution.py
Enhance execution retrieval with PostgreSQLagents-api/agents_api/queries/executions/get_execution.py
get_execution_transition.py
Convert execution transition retrieval to PostgreSQLagents-api/agents_api/queries/executions/get_execution_transition.py
get_paused_execution_token.py
Migrate paused execution token retrieval to PostgreSQLagents-api/agents_api/queries/executions/get_paused_execution_token.py
list_executions.py
Add PostgreSQL-based execution listing functionalityagents-api/agents_api/queries/executions/list_executions.py
lookup_temporal_data.py
Add PostgreSQL-based temporal data lookupagents-api/agents_api/queries/executions/lookup_temporal_data.py
prepare_execution_input.py
Optimize execution input preparation with PostgreSQLagents-api/agents_api/queries/executions/prepare_execution_input.py
update_execution.py
Remove separate execution update functionalityagents-api/agents_api/queries/executions/update_execution.py
create_task_execution.py
Update task execution creation with new database approachagents-api/agents_api/routers/tasks/create_task_execution.py
2 files
fixtures.py
Update test fixtures for PostgreSQL migrationagents-api/tests/fixtures.py
test_execution_queries.py
Convert execution query tests to PostgreSQLagents-api/tests/test_execution_queries.py