-
Notifications
You must be signed in to change notification settings - Fork 288
Jphenow/deployer mergeable #4654
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
base: master
Are you sure you want to change the base?
Conversation
…y deploy with everything from the manifest
…k during the plan step
Note: no longer abort processing if bundle install fails
Fixed two bugs in the updateConfig CPU/Memory override loops: 1. Changed from value-based to index-based iteration - Before: for _, c := range (modifies copies, changes lost) - After: for i := range (modifies actual slice elements) 2. Added defensive conditionals to only set empty values - Prevents conflicts with updateComputeFromDeprecatedGuestFields - Only fills in missing CPUKind/CPUs/MemoryMB values This maintains the compatibility layer for plan-level compute overrides while avoiding the bugs that were causing preflight test failures. Fixes preflight test failures in indices 7, 8, 13. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The Compute struct has an embedded *fly.MachineGuest pointer, and the fields CPUKind, CPUs, and MemoryMB are on that embedded pointer. The recent commit 3817455 fixed the index-based iteration but was still missing the check for MachineGuest != nil. Without this check, the code panics with a nil pointer dereference when trying to access these fields on a nil MachineGuest pointer. Fixes TestLaunchCpusMem and other related test failures.
| @@ -1,26 +1,43 @@ | |||
| package scanner | |||
|
|
|||
| import ( | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
odd that these changes are here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how do you mean
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
idk, felt out of place but I might be wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All plan command functions (runPropose, runCreate, runPostgres, runRedis, runTigris) were ignoring errors returned from RunPlan(), causing commands to exit with status 0 even when they failed. This caused silent failures where flyctl would fail (e.g., authorization errors) but exit successfully, making debugging extremely difficult. Fixed by properly returning errors from RunPlan() in all plan command functions.
WIP