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.

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 t_llm_request is unbounded problem #657

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Jul 29, 2024

🚀 This description was created by Ellipsis for commit 47fa925

Summary:

Fixes unbounded t_llm_request issue in llm_api_handler function by moving its initialization before the try block.

Key points:

  • Fixes unbounded t_llm_request issue in llm_api_handler function.
  • Moves t_llm_request = time.perf_counter() before the try block.
  • Ensures consistency with llm_api_handler_with_router_and_fallback function.
  • Affects skyvern/forge/sdk/api/llm/api_handler_factory.py.

Generated with ❤️ by ellipsis.dev

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 47fa925 in 30 seconds

More details
  • Looked at 19 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern/forge/sdk/api/llm/api_handler_factory.py:193
  • Draft comment:
    The change to move t_llm_request initialization before the try block is correct as it ensures the timing measurement for the API call is accurate.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The change in the PR moves the timing of t_llm_request to before the try block. This is a logical change as it ensures the time is recorded right before the API call is made, rather than after some potentially time-consuming operations. This change is correct and improves the accuracy of the timing measurement for the API call.

Workflow ID: wflow_MkdoJSUUMwTvyF8l


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

@wintonzheng wintonzheng merged commit dd188b5 into main Jul 29, 2024
2 checks passed
@wintonzheng wintonzheng deleted the shu/fix_oubounded_t_llm_request branch July 29, 2024 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant