Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Make transitions queries async #910

Merged
merged 6 commits into from
Nov 29, 2024
Merged

Make transitions queries async #910

merged 6 commits into from
Nov 29, 2024

Conversation

whiterabbit1983
Copy link
Contributor

@whiterabbit1983 whiterabbit1983 commented Nov 29, 2024 •
edited by ellipsis-dev bot
Loading

Important

Make transition queries asynchronous by adding async functions and clients, updating decorators, and fixing minor issues.

  • Async Transition Queries:
    • Added create_execution_transition_async in create_execution_transition.py for async transition creation.
    • Updated transition_step in transition_step.py to use create_execution_transition_async.
  • Async Client:
    • Introduced get_async_cozo_client in cozo.py for async Cozo client.
  • Decorators and Utilities:
    • Added cozo_query_async decorator in utils.py for async query handling.
    • Updated increase_counter in counters.py to support async functions.
  • Miscellaneous:
    • Fixed potential None error in chat.py when formatting tools.
    • Added pycozo-async dependency in pyproject.toml.

This description was created by Ellipsis for 3a32c7c. It will automatically update as commits are pushed.

ellipsis-dev[bot]
ellipsis-dev bot reviewed Nov 29, 2024
View reviewed changes
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.

👍 Looks good to me! Reviewed everything up to c16b572 in 40 seconds

More details
  • Looked at 581 lines of code in 8 files
  • Skipped 1 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. agents-api/agents_api/clients/cozo.py:22
  • Draft comment:
    The get_async_cozo_client function does not handle the cozo_auth option. Consider updating it to include authentication handling similar to get_cozo_client.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
2. agents-api/agents_api/routers/sessions/chat.py:184
  • Draft comment:
    Duplicate code for checking if the developer is paid and session length. Consider refactoring to avoid redundancy.
  • Reason this comment was not posted:
    Comment was on unchanged code.
3. agents-api/agents_api/models/utils.py:430
  • Draft comment:
    Ensure async_wrapper handles exceptions similarly to wrapper for consistent error handling.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The async_wrapper in wrap_in_class should handle exceptions similarly to wrapper to ensure consistent error handling.

Workflow ID: wflow_guvoWKegwLcBOSR9

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

ellipsis-dev[bot]
ellipsis-dev bot reviewed Nov 29, 2024
View reviewed changes
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.

👍 Looks good to me! Incremental review on d20f2e5 in 10 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 1 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. agents-api/pyproject.toml:55
  • Draft comment:
    Ensure that changing pycozo-async from a git URL to a version number does not affect compatibility or functionality. Verify that version ^0.7.6 meets all requirements.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The change from a git URL to a version number for the pycozo-async dependency is a significant change. This should be verified for compatibility and functionality.

Workflow ID: wflow_OYSqf1Aa9M2YwJX7

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

ellipsis-dev[bot]
ellipsis-dev bot reviewed Nov 29, 2024
View reviewed changes
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.

👍 Looks good to me! Incremental review on 9ce2d6b in 11 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. agents-api/agents_api/metrics/counters.py:27
  • Draft comment:
    Ensure that func is a coroutine before using await. The current implementation correctly checks this with inspect.iscoroutinefunction(func) before returning async_wrapper, so the change is appropriate.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The change from return func(*args, **kwargs) to return await func(*args, **kwargs) in the async_wrapper is correct for handling asynchronous functions. However, the async_wrapper should ensure that func is indeed a coroutine function before using await. The current implementation already checks this with inspect.iscoroutinefunction(func) before returning async_wrapper, so the change is appropriate.

Workflow ID: wflow_8Z3UeM42NMV1cyLb

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

ellipsis-dev[bot]
ellipsis-dev bot reviewed Nov 29, 2024
View reviewed changes
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.

👍 Looks good to me! Incremental review on 3a32c7c in 34 seconds

More details
  • Looked at 72 lines of code in 5 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. agents-api/agents_api/activities/task_steps/prompt_step.py:1
  • Draft comment:
    Callable import removed but still used in commented code. Re-add if uncommenting later.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The import statement for Callable in prompt_step.py is removed but still used in the commented-out code. If the commented code is to be used later, this import might be necessary.
2. agents-api/agents_api/autogen/Tasks.py:9
  • Draft comment:
    Unused import JinjaTemplate removed. This is a valid change.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The import statement for JinjaTemplate in Tasks.py is removed but not used anywhere else in the file. This is a valid removal.
3. agents-api/tests/sample_tasks/simple_multi_step.yaml:23
  • Draft comment:
    Modified evaluate step to handle multiple items. This aligns with async transition intent.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The change in simple_multi_step.yaml modifies the evaluate step to handle multiple items. This change seems correct and aligns with the intent to make transitions queries async.

Workflow ID: wflow_htQNTImBWSaKaQeE

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@creatorrr creatorrr merged commit d3b37db into dev Nov 29, 2024
4 checks passed
@creatorrr creatorrr deleted the f/async-transition branch November 29, 2024 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@standard-input standard-input[bot] standard-input[bot] left review comments

@ellipsis-dev ellipsis-dev[bot] ellipsis-dev[bot] left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@whiterabbit1983 @creatorrr