Skip to content

optionally generate presigned urls - executions api#324

Open
rootflo-hardik wants to merge 1 commit into
developfrom
fix/generate_presigned_urls_optionally_executions_api
Open

optionally generate presigned urls - executions api#324
rootflo-hardik wants to merge 1 commit into
developfrom
fix/generate_presigned_urls_optionally_executions_api

Conversation

@rootflo-hardik

@rootflo-hardik rootflo-hardik commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Performance Improvements

    • Improved execution listings by avoiding unnecessary file URL generation when displaying multiple executions.
  • Bug Fixes

    • Improved handling of output, history, and input file links in execution status responses.
    • Added validation to prevent invalid input file records from causing URL-generation errors.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds conditional presigned URL generation to status responses and disables URL generation when listing multiple executions.

Changes

Presigned URL generation

Layer / File(s) Summary
Status response URL gating
wavefront/server/modules/agents_module/agents_module/services/async_agentic_execution_service.py
_build_status_response conditionally generates output, history, and input-file URLs, validates input-file keys, and list_executions disables URL generation for bulk results.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: making presigned URL generation optional in the executions API.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/generate_presigned_urls_optionally_executions_api

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
wavefront/server/modules/agents_module/agents_module/services/async_agentic_execution_service.py (2)

429-432: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add regression coverage for the bulk no-URL contract.

Assert that list_executions does not call generate_presigned_url, returns output_url and history_url as None, and leaves input-file url fields unset; also verify that individual status responses still generate URLs.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@wavefront/server/modules/agents_module/agents_module/services/async_agentic_execution_service.py`
around lines 429 - 432, Add regression tests around list_executions and the
individual status-response path using _build_status_response: verify bulk
responses never call generate_presigned_url, expose output_url and history_url
as None, and leave input-file url fields unset, while individual status
responses continue generating URLs.

352-365: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Narrow the per-file presigned-URL exception handler.

The current handlers catch every Exception, while the storage implementations raise different provider-specific errors during URL generation. Catch the documented provider exception types instead of swallowing malformed storage-client usage and avoid losing the first input-file key in the log.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@wavefront/server/modules/agents_module/agents_module/services/async_agentic_execution_service.py`
around lines 352 - 365, Update the per-file URL generation in the input-file
loop of the async execution service to catch only the documented
provider-specific exceptions raised by cloud_storage.generate_presigned_url,
preserving unexpected storage-client errors for propagation. Include the current
input_file key in the warning so failures identify the affected file, while
retaining processing of subsequent files.

Source: Linters/SAST tools

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@wavefront/server/modules/agents_module/agents_module/services/async_agentic_execution_service.py`:
- Around line 429-432: Add regression tests around list_executions and the
individual status-response path using _build_status_response: verify bulk
responses never call generate_presigned_url, expose output_url and history_url
as None, and leave input-file url fields unset, while individual status
responses continue generating URLs.
- Around line 352-365: Update the per-file URL generation in the input-file loop
of the async execution service to catch only the documented provider-specific
exceptions raised by cloud_storage.generate_presigned_url, preserving unexpected
storage-client errors for propagation. Include the current input_file key in the
warning so failures identify the affected file, while retaining processing of
subsequent files.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6b91b57c-1b14-4059-be05-849e77c82b71

📥 Commits

Reviewing files that changed from the base of the PR and between f9ee9cd and 881e6ca.

📒 Files selected for processing (1)
  • wavefront/server/modules/agents_module/agents_module/services/async_agentic_execution_service.py

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