Skip to content

Revert "fix: use uv for modal deployment"#528

Merged
ColeMurray merged 1 commit intomainfrom
revert-509-use-uv-for-modal-deployments
Apr 21, 2026
Merged

Revert "fix: use uv for modal deployment"#528
ColeMurray merged 1 commit intomainfrom
revert-509-use-uv-for-modal-deployments

Conversation

@ColeMurray
Copy link
Copy Markdown
Owner

@ColeMurray ColeMurray commented Apr 21, 2026

Reverts #509

Summary by CodeRabbit

  • Refactor
    • Streamlined the deployment process by removing unnecessary validation checks and simplifying command execution, improving deployment efficiency.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 21, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e24f8f88-c034-4e93-8aff-e6aa3764c40c

📥 Commits

Reviewing files that changed from the base of the PR and between f816a52 and 380f49d.

📒 Files selected for processing (1)
  • terraform/modules/modal-app/scripts/deploy.sh

📝 Walkthrough

Walkthrough

The deployment script simplifies its modal invocation by removing preflight checks for the uv package manager and pyproject.toml file. Direct invocation of modal commands replaces the previous uv run modal approach, while preserving existing error handling and deployment logic across all deployment module branches.

Changes

Cohort / File(s) Summary
Deployment Script Simplification
terraform/modules/modal-app/scripts/deploy.sh
Removed uv installation verification, removed pyproject.toml validation check, and eliminated MODAL_CMD array construction. Changed deployment invocation from uv run modal to direct modal command execution; error handling pattern unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • fix: use uv for modal deployment #509: Introduces the uv-based invocation and preflight checks that are now being removed in this PR, representing a reversal of that approach in favor of direct modal invocation.

Poem

🐰 Hop, hop away with uv's array,
No checks to weigh us down today,
Modal stands alone and free,
Deployed swift as a rabbit's spree!

✨ 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 revert-509-use-uv-for-modal-deployments

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 and usage tips.

@ColeMurray ColeMurray merged commit d7b8e8a into main Apr 21, 2026
7 of 8 checks passed
@ColeMurray ColeMurray deleted the revert-509-use-uv-for-modal-deployments branch April 21, 2026 18:05
@github-actions
Copy link
Copy Markdown

Terraform Validation Results

Step Status
Format
Init
Validate

Note: Terraform plan was skipped because secrets are not configured. This is expected for external contributors. See docs/GETTING_STARTED.md for setup instructions.

Pushed by: @ColeMurray, Action: pull_request

Copy link
Copy Markdown
Contributor

@open-inspect open-inspect Bot left a comment

Choose a reason for hiding this comment

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

Summary

This PR reverts the recent switch from modal deploy to uv run modal in terraform/modules/modal-app/scripts/deploy.sh. The change looks correct: the Terraform deployment workflow currently installs modal directly with pip and does not provision uv, so this revert restores compatibility with the documented and automated deployment path.

  • PR Title and number: Revert "fix: use uv for modal deployment" (#528)
  • Author: @ColeMurray
  • Files changed count, additions/deletions: 1 file changed, 3 additions / 15 deletions

Critical Issues

None.

Suggestions

None.

Nitpicks

None.

Positive Feedback

  • The revert keeps the deploy script aligned with the repository documentation, which consistently uses modal deploy.
  • The change also matches the Terraform GitHub Actions workflow, where modal is installed explicitly but uv is not.
  • The scope is minimal and isolated to the failing integration point, which keeps the rollback low risk.

Questions

None.

Verdict (for GitHub PRs)

Approve: Ready to merge, no blocking issues.

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