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 recording_url #1818

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix recording_url #1818

wants to merge 2 commits into from

Conversation

mingfang
Copy link

@mingfang mingfang commented Feb 24, 2025

fixes #1653


Important

Fixes recording_url handling in WorkflowRunRecording.tsx by converting file:// URLs to API-accessible URLs.

  • Behavior:
    • Fixes recording_url handling in WorkflowRunRecording.tsx.
    • Converts file:// URLs to API-accessible URLs using artifactApiBaseUrl.
  • Imports:
    • Adds import for artifactApiBaseUrl from @/util/env.

This description was created by Ellipsis for 4ecd480. 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.

❌ Changes requested. Reviewed everything up to 4ecd480 in 1 minute and 15 seconds

More details
  • Looked at 18 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. skyvern-frontend/src/routes/workflows/workflowRun/WorkflowRunRecording.tsx:8
  • Draft comment:
    Consider URL-encoding the file path to avoid issues with spaces or special characters.
  • Reason this comment was not posted:
    Confidence changes required: 50% <= threshold 50%
    None
2. skyvern-frontend/src/routes/workflows/workflowRun/WorkflowRunRecording.tsx:8
  • Draft comment:
    Ensure artifactApiBaseUrl doesn't accidentally introduce duplicate or missing slashes in the URL.
  • Reason this comment was not posted:
    Confidence changes required: 40% <= threshold 50%
    None
3. skyvern-frontend/src/routes/workflows/workflowRun/WorkflowRunRecording.tsx:7
  • Draft comment:
    Consider adding an inline comment explaining the rationale behind converting file:// URLs to the artifact API endpoint. This could help future developers understand why the transformation is needed.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None

Workflow ID: wflow_AdQNHeUfyRF69Hc2


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

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.

Media URLs seem incorrect when not local
2 participants