Skip to content

fix(scripts): append database URLs instead of sed replacement #334

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.

Sign up for GitHub

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

Merged
saddlepaddle merged 1 commit into from
Dec 12, 2025
Merged

fix(scripts): append database URLs instead of sed replacement #334

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions .superset/setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,15 @@ else
POOLED_URL=$(neonctl connection-string "$BRANCH_ID" --project-id "$NEON_PROJECT_ID" --pooled)
fi

# Copy root .env and override with branch-specific values
# Copy root .env and append branch-specific values
cp "$SUPERSET_ROOT_PATH/.env" .env
sed -i '' "s|^DATABASE_URL=.*|DATABASE_URL=$POOLED_URL|" .env
sed -i '' "s|^DATABASE_URL_UNPOOLED=.*|DATABASE_URL_UNPOOLED=$DIRECT_URL|" .env
echo "NEON_BRANCH_ID=$BRANCH_ID" >> .env
cat >> .env << EOF

# Workspace Database (Neon Branch)
NEON_BRANCH_ID=$BRANCH_ID
DATABASE_URL=$POOLED_URL
DATABASE_URL_UNPOOLED=$DIRECT_URL
EOF
Comment on lines +71 to +79
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Dec 12, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

“Append” may not actually override: duplicated keys in .env are parsed inconsistently across dotenv loaders.
This change intentionally avoids sed-replacement, but now relies on whatever reads .env to pick the last occurrence of DATABASE_URL / DATABASE_URL_UNPOOLED / NEON_BRANCH_ID. Some consumers treat first definition as authoritative, which would silently ignore the appended block.

If you want “append without depending on exact formatting” and deterministic override, consider filtering those keys out before appending:

-# Copy root .env and append branch-specific values
-cp "$SUPERSET_ROOT_PATH/.env" .env
+# Copy root .env, drop any existing workspace DB keys, then append branch-specific values
+grep -vE '^(NEON_BRANCH_ID|DATABASE_URL|DATABASE_URL_UNPOOLED)=' "$SUPERSET_ROOT_PATH/.env" > .env
 cat >> .env << EOF
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Copy root .env and append branch-specific values
cp "$SUPERSET_ROOT_PATH/.env" .env
sed -i '' "s|^DATABASE_URL=.*|DATABASE_URL=$POOLED_URL|" .env
sed -i '' "s|^DATABASE_URL_UNPOOLED=.*|DATABASE_URL_UNPOOLED=$DIRECT_URL|" .env
echo "NEON_BRANCH_ID=$BRANCH_ID" >> .env
cat >> .env << EOF
# Workspace Database (Neon Branch)
NEON_BRANCH_ID=$BRANCH_ID
DATABASE_URL=$POOLED_URL
DATABASE_URL_UNPOOLED=$DIRECT_URL
EOF
# Copy root .env, drop any existing workspace DB keys, then append branch-specific values
grep -vE '^(NEON_BRANCH_ID|DATABASE_URL|DATABASE_URL_UNPOOLED)=' "$SUPERSET_ROOT_PATH/.env" > .env
cat >> .env << EOF
# Workspace Database (Neon Branch)
NEON_BRANCH_ID=$BRANCH_ID
DATABASE_URL=$POOLED_URL
DATABASE_URL_UNPOOLED=$DIRECT_URL
EOF
🤖 Prompt for AI Agents 
In .superset/setup.sh around lines 71 to 79, appending branch-specific variables
to .env can produce duplicate keys that some dotenv parsers treat as
authoritative on the first occurrence; instead, remove any existing
NEON_BRANCH_ID, DATABASE_URL and DATABASE_URL_UNPOOLED entries from the target
.env before appending so the appended values are the only definitions. Implement
this by creating a temporary file that filters out lines starting with those
keys (preserving other content), then overwrite .env with the filtered result
and append the branch block; ensure you handle edge cases like missing .env and
preserve file permissions.

Comment on lines +73 to +79
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Dec 12, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Consider quoting/escaping .env values to avoid edge-case parsing breaks.
If a connection string ever contains characters that some dotenv parsers treat specially (notably whitespace or #), the unquoted KEY=$VALUE form can break.

At minimum, consider:

-DATABASE_URL=$POOLED_URL
-DATABASE_URL_UNPOOLED=$DIRECT_URL
+DATABASE_URL="${POOLED_URL}"
+DATABASE_URL_UNPOOLED="${DIRECT_URL}"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
cat >> .env << EOF
# Workspace Database (Neon Branch)
NEON_BRANCH_ID=$BRANCH_ID
DATABASE_URL=$POOLED_URL
DATABASE_URL_UNPOOLED=$DIRECT_URL
EOF
cat >> .env << EOF
# Workspace Database (Neon Branch)
NEON_BRANCH_ID=$BRANCH_ID
DATABASE_URL="${POOLED_URL}"
DATABASE_URL_UNPOOLED="${DIRECT_URL}"
EOF
🤖 Prompt for AI Agents 
In .superset/setup.sh around lines 73-79, the here-doc writes unquoted env
values which can break dotenv parsers if values contain whitespace or #; change
the write to quote and escape values before appending: build safe variables by
escaping any single quotes (e.g. safe_pooled=$(printf "%s" "$POOLED_URL" | sed
"s/'/'\\\\''/g") and safe_direct=$(printf "%s" "$DIRECT_URL" | sed
"s/'/'\\\\''/g")), then write DATABASE_URL='$safe_pooled' and
DATABASE_URL_UNPOOLED='$safe_direct' into .env (or use printf '%s\n'
"DATABASE_URL='$safe_pooled'" "DATABASE_URL_UNPOOLED='$safe_direct'") so values
are wrapped in single quotes and internal quotes are escaped.


success "Neon branch created: $WORKSPACE_NAME"
echo "✨ Done!"