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

Integrate v2 task api #1566

Merged
merged 1 commit into from
Jan 15, 2025
Merged

Integrate v2 task api #1566

merged 1 commit into from
Jan 15, 2025

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Jan 15, 2025

Important

Integrate v2 task API by adding a new Axios client, updating API calls, and renaming types and fields to align with the new API structure.

  • API Client:
    • Add v2Client in AxiosClient.ts with base URL apiV2BaseUrl.
    • Update getClient() to return v2Client for version "v2".
  • Type Updates:
    • Rename ObserverCruise to ObserverTask in types.ts.
    • Rename observer_cruise to observer_task in WorkflowRunStatusApiResponse.
    • Rename observer_thought_id to thought_id in workflowRunTypes.ts.
  • Component Updates:
    • Update PromptBox.tsx to use v2Client for task creation.
    • Update WorkflowRun.tsx, WorkflowRunOverview.tsx, and WorkflowRunTimeline.tsx to use thought_id.
    • Update WorkflowPostRunParameters.tsx and WorkflowRunOutput.tsx to use observer_task instead of observer_cruise.

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

…src/'

<!-- ELLIPSIS_HIDDEN -->

> [!IMPORTANT]
> Integrate v2 task API into the frontend, updating API clients, renaming variables, and modifying components to support new task handling logic.
>
>   - **API Integration**:
>     - Introduced `v2Client` in `AxiosClient.ts` for v2 API integration.
>     - Updated `getClient()` to support version selection (default to v1).
>   - **Renaming**:
>     - Renamed `observer_cruise` to `observer_task` in `WorkflowPostRunParameters.tsx`, `WorkflowRunOutput.tsx`, and `types.ts`.
>     - Renamed `observer_thought_id` to `thought_id` in `WorkflowRun.tsx`, `WorkflowRunOverview.tsx`, and `WorkflowRunTimeline.tsx`.
>   - **Components and Hooks**:
>     - Updated `PromptBox.tsx` to use v2 API for task creation.
>     - Modified `WorkflowRun.tsx` and `WorkflowRunTimeline.tsx` to handle new task and thought identifiers.
>   - **Miscellaneous**:
>     - Added cost display in `WorkflowRunTimeline.tsx`.
>     - Updated `findActiveItem()` and `findBlockSurroundingAction()` in `workflowTimelineUtils.ts` to accommodate new identifiers.
>
> <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 120b5365d6119a1440331a30a0a2eefa7f87ade3. It will automatically update as commits are pushed.</sup>

<!-- ELLIPSIS_HIDDEN -->
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 ef58dbd in 1 minute and 21 seconds

More details
  • Looked at 240 lines of code in 10 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. skyvern-frontend/src/api/AxiosClient.ts:32
  • Draft comment:
    The removeAuthorizationHeader function only checks if the Authorization header is present in client before deleting it from both client and v2Client. Consider checking for the header in both clients separately to avoid unintended deletions.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment raises a valid point about potential unintended deletions, but is it actually a problem? Both clients are initialized the same way, and headers are always set on both clients simultaneously in setAuthorizationHeader(). There's no way for them to get out of sync. The check is just an optimization to avoid unnecessary deletions when no header exists.
    The comment identifies a theoretical edge case where the clients could have different header states. What if some other code modifies the headers directly?
    The entire file shows a consistent pattern of treating both clients' headers identically. The API design enforces this through the exported functions, making direct header modification unlikely and unsupported.
    The comment should be deleted as it suggests a defensive coding pattern that adds complexity without providing meaningful protection against realistic scenarios.
2. skyvern-frontend/src/api/AxiosClient.ts:44
  • Draft comment:
    The removeApiKeyHeader function only checks if the X-API-Key header is present in client before deleting it from both client and v2Client. Consider checking for the header in both clients separately to avoid unintended deletions.
  • Reason this comment was not posted:
    Marked as duplicate.

Workflow ID: wflow_zDSVZej4hmzQKSKr


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

@msalihaltun msalihaltun merged commit 9cd542e into main Jan 15, 2025
6 checks passed
@msalihaltun msalihaltun deleted the salih/integrate-v2-tasks branch January 15, 2025 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants