fix: experiment workers not set up in pushkin-cli currently#376
Merged
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
🦋 Changeset detectedLatest commit: cb3be45 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
cherriechang
added
enhancement
This commit adds comprehensive Auth0 authentication support to the basic site template, enabling OAuth2/OIDC authentication for Pushkin users. The implementation is backward-compatible and only activates when useAuth is enabled in pushkin.yaml.
Key Changes:
- Added Auth0 authentication components (Login, Logout, Profile)
- Added AuthSync component to synchronize Auth0 state with Redux
- Enhanced Redux infrastructure to handle both Auth0 and session-based auth
- Updated Header component with conditional auth buttons and 'My account' link
- Added conditional Auth0Provider wrapper in index.js
- Created config.js to expose authDomain and authClientID from pushkin.yaml
- Added @auth0/auth0-react dependency to package.json
- Added /profile route to App.js
Features:
- Dual authentication modes: Auth0 (when configured) and session-based (fallback)
- Conditional rendering based on useAuth config setting
- Seamless integration with existing Redux state management
- User profile management via Auth0 dashboard
- Automatic token refresh and localStorage caching
Configuration:
Sites can enable Auth0 by setting in pushkin.yaml:
addons:
useAuth: true
authDomain: your-domain.auth0.com
authClientID: your-client-id
All changes are backward-compatible. Sites without Auth0 configuration will continue using session-based authentication.
- Header's useEffect now skips dispatching getUser() when Auth0 is enabled - Auth0 users are managed by AuthSync component via SET_AUTH0_USER action - This prevents session-based user IDs from replacing Auth0 user IDs in Redux 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add heartbeat: 30 configuration to amqp.connect() in pushkin-worker - Upgrade RabbitMQ from 3.6 to 3.12 in docker-compose template - Add RABBITMQ_HEARTBEAT environment variable This fixes the "Heartbeat timeout" error that was preventing experiment workers from completing database operations. The 30-second heartbeat interval ensures the connection stays alive during long-running tasks.
…ections - Fix "Cannot read properties of undefined" error by checking if worker service exists - Use 'test_db' and 'test_transaction_db' Docker service names instead of localhost - Use internal port 5432 instead of host-mapped port for TRANS_PORT - Fixes ECONNREFUSED errors when workers try to connect to databases This ensures workers can communicate with databases via Docker networking.
session_id persistence in users database
cherriechang
force-pushed
the
fix/experiment-workers
branch
from
November 26, 2025 22:20
06b173a to
e2c178d
Compare
…TaskCreator() Problem: When processing multiple workers (e.g., ew_test_worker, ew_test_2_worker): 1. First iteration created a reference to the template object 2. Modified the Docker image property (which mutated the shared template) 3. Second iteration reused the same mutated template object 4. Result: All workers ended up with the last processed worker's Docker image This caused deployment failures where: - ew_test_worker service used ew_test_2_worker:latest image (wrong!) - Workers consumed from incorrect RabbitMQ queues - Experiments failed to respond to API requests
Collaborator
|
@cherriechang -- what should I be looking at for this pull request? (I'm prioritizing getting through these pull requests) |
Contributor
Author
Currently everything is tested in an end-to-end way (this goes for the fix/aws-deployment and feature/authentication-system branches too) -- by making a new test website and seeing if the target feature works 😅 I should add unit tests to this PR before merging it, but if you wanted to you could test the current changes out by making a new test Pushkin site using the version of Pushkin on this branch, adding an experiment with some minimal javascript and seeing if it reaches end of experiment smoothly! Problems to look out for include it getting stuck at a screen that says "Loading...", data not getting saved to the main database, and/or these things failing when user profile/authentication is enabled. |
Contributor
…s nothing (not only for verbose)
… conflicts: - aws/index.js: kept structuredClone (fix/aws-deployment) over JSON.parse(JSON.stringify()) — both fix the same worker-sharing bug; structuredClone is cleaner. - prep/index.js: kept hardcoded Docker service names test_db/test_transaction_db (fix/experiment-workers) over pushkinYAML.databases.*.host — localtestdb has no host field in pushkin.yaml, so the fix/aws-deployment version would have set DB_HOST=undefined. Will fix in separate commit. - templates/experiments/basic/src/worker/index.js: kept SSL auto-detection via pushkin-worker (fix/experiment-workers) over DB_SSL env var — pushkin-worker now auto-detects SSL for RDS endpoints, so template workers don't need to configure it.
…me is set in one place
…(leftover from a different solution that was removed in favor of SSL auto-detection)
becky-gilbert
approved these changes
Apr 8, 2026
becky-gilbert
left a comment
becky-gilbert
left a comment
Contributor
There was a problem hiding this comment.
This looks ready to merge! 🎉 There are some unrelated issues to fix, and I need to work off of this branch to fix those issues. So I think we should merge into fix/aws-deployment so that I can continue to use that as the base branch to merge those fixes into.
Here's what I changed:
- b46d185 - always print the error about not being able to check for existing containers (not just with verbose)
- 0bc9e5f - fix undefined var e in catch block (pre-existing bug)
- 55c4cb3 - In
setupdbensureCleanState, switch fromcompose stop/compose rmtocompose down. This does same thing but also removes named volumes. This is related to a separate issue (#372), but I fixed it here because it was blocking my workflow. - 6a31819 - Fix merge conflicts with
fix/aws-deployment. I had fixed some issues in that branch that were also fixed here, so I went through these and found the better of the two fixes.aws/index.js: keptstructuredCloneoverJSON.parse(JSON.stringify())prep/index.js: kept hardcoded Docker service names (just for this merge commit, changed to variables in separate commit)templates/experiments/basic/src/worker/index.js: kept SSL auto-detection via pushkin-worker from this branch overDB_SSLenv var.
- 7e4b3a1 - Use
DB_HOST/TRANS_HOSTvalues frompushkinYAMLso that the docker service name is set in one place. - 929ecb3 - Remove references to old
DB_SSLenv variable andsslConfigin workers (missed these in the merge commit) - 0c50a41 - Merge
fix/prep-graceful-handle-container-existto fix db password issue, and fix merge conflicts. - cb3be45 - The AMQP connection heartbeat needs to be set via query parameter
Here's the testing that I've done using a new test site created with this build, and with several experiments.
- Verified
DB_HOST/DB_TRANSvalues are correct - Docker build still works following switch to
execFile - Heartbeat is set correctly: RabbitMQ container shows 30s timeout (
docker exec pushkin-message-queue-1 rabbitmqctl list_connections name timeout) - Heartbeat works: no timeout errors due to idle period during experiment
- Experiment data persists
- Experiments are no longer sharing the same worker config (fixed via
structuredClone) - DB SSL is auto-detected correctly: SSL is used with the RDS endpoint hostname, and SSL is not used locally
useIAMprofile strings are accessed correctly, and an invalid profile name produces a clear and immediate error (due to awaitingcheckIAMUser)
One thing I couldn't verify is the retry logic for PutBucketPolicy because it worked on the first attempt every time I ran aws init.
@cherriechang and I decided to leave out AWS-related unit tests for the time being. They're too difficult to write because the components we need to test are not modularized or exported from index.js. They will be modularized via Cherrie's refactoring (#374) so we should add tests for that branch when it's ready.
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
4 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.
This should enable properly running an experiment and storing its data.
Also needed to verify user authentication system #375