Skip to content

fix(bench): provide explicit Fly build-only config - #1001

Merged
DecisionNerd merged 1 commit into
mainfrom
fix/958-fly-build-config
Aug 30, 2026
Merged

fix(bench): provide explicit Fly build-only config#1001
DecisionNerd merged 1 commit into
mainfrom
fix/958-fly-build-config

Conversation

@DecisionNerd

@DecisionNerd DecisionNerd commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a checked-in minimal Fly build-only config for the filesystem qualification Dockerfile
  • keep the config identity-free and service-free; supply the unique disposable app only via runtime --app
  • pass absolute build-context, config, and Dockerfile paths so execution is independent of the caller working directory
  • preserve remote-only, build-only, push, no-public-IPs, commit build argument, and immutable same-app digest behavior
  • prove a build failure occurs before volume or Machine creation and still tears down

Root cause evidence

After PR #1000, app readiness converged in the authorized post-fix attempt, but flyctl deploy --build-only still returned typed build_failed before image resolution. The repository had no fly.toml and the executor supplied no explicit --config, while Fly deploy consumes app configuration from that path.

Validation

  • make -C benchmarks fly-adapter-static — 29 passed
  • make -C benchmarks smoke-python — smoke passed; 83 passed
  • changed-file Ruff format and lint — passed
  • python3 scripts/ci/test-fly-filesystem-safety-contract.py — 2 passed
  • checked-in TOML parses to only the expected build table
  • git diff --check — passed

Issue

Relates to issue 958. It remains open; a merged-executor tiny rerun with qualified evidence and independent teardown remains the completion gate.

No Fly resources were created by this change.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by CodeRabbit

  • New Features

    • Added Fly deployment support for explicitly supplied build configuration, source, and Dockerfile paths.
    • Added a dedicated Fly build configuration for qualification deployments.
    • Qualification deployments now use disposable app names supplied at deployment time.
  • Bug Fixes

    • Invalid relative build paths are now rejected.
    • Qualification builds enforce private, build-only deployments without creating volumes or machines.

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 60568e22-be0f-47c9-8fda-a699372727ce

📥 Commits

Reviewing files that changed from the base of the PR and between f1606c2 and 7618f8f.

⛔ Files ignored due to path filters (1)
  • benchmarks/README.md is excluded by !**/*.md
📒 Files selected for processing (5)
  • benchmarks/harness/graphforge_bench/fly_adapter.py
  • benchmarks/harness/graphforge_bench/fly_tiny_qualification.py
  • benchmarks/tests/test_fly_adapter.py
  • benchmarks/tests/test_fly_tiny_qualification.py
  • containers/fly-filesystem-qualification/fly.build.toml

Included review availability: 1 review is currently available. Your included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour.


Walkthrough

The Fly adapter now requires an absolute build configuration path and passes it to flyctl deploy. The tiny qualification workflow uses a build-only configuration that selects its Dockerfile. Tests validate path handling, command flags, and configuration contents.

Changes

Fly build configuration

Layer / File(s) Summary
Adapter contract and deploy command
benchmarks/harness/graphforge_bench/fly_adapter.py, benchmarks/tests/test_fly_adapter.py
remote_build_command requires absolute source, config, and dockerfile paths. The generated command includes --config. Tests cover propagation and rejection of relative paths.
Qualification build wiring and validation
benchmarks/harness/graphforge_bench/fly_tiny_qualification.py, containers/fly-filesystem-qualification/fly.build.toml, benchmarks/tests/test_fly_tiny_qualification.py
The qualification build passes fly.build.toml, which selects the Dockerfile and excludes app, service, and HTTP identity settings. Tests validate deploy flags, commit metadata, private networking, and the absence of volume or machine creation.

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

Merge Risk: ⚪ Minimal · up to 7618f

The PR adds an explicit, identity-free Fly build-only configuration and makes execution independent of the caller’s working directory while preserving the existing build behavior; validation checks pass, and no actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 4 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding an explicit Fly build-only configuration for the benchmark.
Description check ✅ Passed The description provides a clear summary, root cause, implementation details, validation results, issue reference, and resource impact. It does not use every template heading or checklist item, but th…
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.
Full details: Description check

Explanation

The description provides a clear summary, root cause, implementation details, validation results, issue reference, and resource impact. It does not use every template heading or checklist item, but the core required information is complete.

Full details: Docstring Coverage

Explanation

Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 4 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/958-fly-build-config

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.


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

@github-actions github-actions Bot added documentation Improvements or additions to documentation release:none No release note or version impact labels Aug 30, 2026
@DecisionNerd

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@DecisionNerd
DecisionNerd merged commit 95887ce into main Aug 30, 2026
21 checks passed
@DecisionNerd
DecisionNerd deleted the fix/958-fly-build-config branch August 30, 2026 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation release:none No release note or version impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant