Skip to content

Conversation

joaquim-verges
Copy link
Member

@joaquim-verges joaquim-verges commented Oct 3, 2025

Summary by CodeRabbit

  • New Features
    • Added support for AWS-based authentication in benchmarks via optional AWS credentials (Access Key ID, Secret Access Key, KMS ARN).
    • Made the vault access token optional; benchmarks can now run with either Vault or AWS credentials.
    • Requests now automatically include only the relevant authentication headers based on provided credentials.
    • Improved validation to require only essential inputs and guide setup accordingly.
    • Minor logging and console output tweaks for clearer run feedback (no behavior changes beyond conditional headers).

Copy link

coderabbitai bot commented Oct 3, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Validation now treats only FROM and SECRET_KEY as required. BenchmarkConfig makes vaultAccessToken optional and adds optional AWS fields (awsAccessKeyId, awsKmsArn, awsSecretAccessKey). HTTP headers are conditionally added based on presence of vault token or all AWS fields. Minor formatting, logging, and type annotations updated.

Changes

Cohort / File(s) Summary of Changes
Benchmark config and request header updates
scripts/benchmarks/eoa.ts
Made BenchmarkConfig.vaultAccessToken optional; added optional AWS fields (awsAccessKeyId, awsKmsArn, awsSecretAccessKey). Updated validation to require only FROM and SECRET_KEY. Added conditional HTTP headers: include vault token if present; include AWS headers only when all three AWS fields are provided. Minor logging/formatting and type annotation adjustments.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant User as CLI/Runner
  participant Script as eoa.ts
  participant API as Benchmark Endpoint

  User->>Script: Provide config (FROM, SECRET_KEY, vaultAccessToken?, AWS creds?)
  rect rgba(200,220,255,0.25)
    note right of Script: Validation (changed)
    Script->>Script: Check FROM, SECRET_KEY (required)
    Script->>Script: Read vaultAccessToken? (optional)
    Script->>Script: Read AWS fields? (all 3 required together)
  end

  rect rgba(220,255,220,0.25)
    note right of Script: Build headers (conditional)
    Script->>Script: Base headers
    alt vaultAccessToken present
      Script->>Script: Add X-Vault-Access-Token
    end
    alt all AWS fields present
      Script->>Script: Add X-AWS-Access-Key-Id<br/>Add X-AWS-Secret-Access-Key<br/>Add X-AWS-KMS-ARN
    end
  end

  Script->>API: POST /webhook with headers+payload
  API-->>Script: JSON response
  Script->>Script: Parse JSON (type annotations updated)
  Script-->>User: Log results
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch Add_AWS_KMS_support_to_EOA_benchmark_script

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between bcc5c4c and 76aa1a9.

📒 Files selected for processing (1)
  • scripts/benchmarks/eoa.ts (15 hunks)

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

@joaquim-verges joaquim-verges marked this pull request as ready for review October 3, 2025 08:17
Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@joaquim-verges joaquim-verges merged commit 47dd4fd into main Oct 3, 2025
3 of 4 checks passed
@joaquim-verges joaquim-verges deleted the Add_AWS_KMS_support_to_EOA_benchmark_script branch October 3, 2025 08:17
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