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

do not use status as a param in logs - use step_status and task_status #1705

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Feb 3, 2025

Important

Rename status to step_status and task_status in agent.py and block.py for clearer logging.

  • Behavior:
    • Rename status to step_status in update_step() in agent.py.
    • Rename status to task_status in update_task() in agent.py.
    • Rename status to task_status in a logging statement in execute() in block.py.

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

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 0a3ca6f in 30 seconds

More details
  • Looked at 35 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 4 drafted comments based on config settings.
1. skyvern/forge/sdk/workflow/models/block.py:1
  • Draft comment:
    No issues: This file does not use 'status' as a log parameter inappropriately. Everything looks good.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    None
2. skyvern/forge/agent.py:1927
  • Draft comment:
    Good change – using 'step_status' instead of 'status' in logging improves clarity. Ensure that downstream log parsing is updated accordingly.
  • Reason this comment was not posted:
    Comment was not on a location in the diff, so it can't be submitted as a review comment.
3. skyvern/forge/agent.py:1980
  • Draft comment:
    Similarly, replacing 'status' with 'task_status' in update_task logging is clear. Confirm that all components reflecting task status are updated.
  • Reason this comment was not posted:
    Comment did not seem useful.
4. skyvern/forge/sdk/workflow/models/block.py:1
  • Draft comment:
    No changes needed here. The file follows best practices and uses descriptive parameter names. Ensure consistency with updated log parameter names if referenced in related modules.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    None

Workflow ID: wflow_fsKVQhDR6kLd7g22


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

@wintonzheng wintonzheng merged commit eb0d4db into main Feb 3, 2025
7 checks passed
@wintonzheng wintonzheng deleted the shu/update_step_status_in_duration_metrics branch February 3, 2025 14:28
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