fix: pushkin-prep should gracefully handle containers already existing and non-Pushkin templates#372
Open
cherriechang wants to merge 1 commit intomainfrom
Open
fix: pushkin-prep should gracefully handle containers already existing and non-Pushkin templates#372cherriechang wants to merge 1 commit intomainfrom
cherriechang wants to merge 1 commit intomainfrom
Conversation
When running `pushkin-dev prep` with existing database containers from a previous run, the command would fail with a misleading "password authentication failed" error. This happened because the existing containers were using different credentials than the current pushkin.yaml config. This commit implements Option 1 from the issue: automatic detection and cleanup of stale database containers. Changes: - Added `ensureCleanState()` function that checks for existing database containers before setup - If containers exist, displays a warning and removes them cleanly - Uses `docker ps -a` to detect both running and stopped containers - Follows the same pattern as `killLocal()` using compose.stop() and compose.rm() - Includes proper error handling to avoid failing the entire setup Benefits: - Makes `prep` command idempotent - safe to run multiple times - Eliminates confusing password authentication errors - Improves developer experience by avoiding manual cleanup steps - No waiting through 10 failed connection attempts (~2+ minutes) Fixes pushkin-dev prep fails cryptically when database containers already exist
|
This was
linked to
issues
Nov 24, 2025
cherriechang
changed the title
cherriechang
changed the title
|
@cherriechang I've been hitting this issue while reviewing #373, so it would be nice to merge this ASAP! Is it ready for review? |
Author
I think so! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1) Failing for existing container
When running
pushkin-dev prepwith existing database containers from a previous run, the command would fail with a misleading "password authentication failed" error. This happened because the existing containers were using different credentials than the current pushkin.yaml config.This commit implements Option 1 from the issue: automatic detection and cleanup of stale database containers.
Changes:
ensureCleanState()function that checks for existing database containers before setupdocker ps -ato detect both running and stopped containerskillLocal()using compose.stop() and compose.rm()Benefits:
prepcommand idempotent - safe to run multiple timesFixes pushkin-dev prep fails cryptically when database containers already exist
2) Failing for non-Pushkin site templates