Fix MySQL commands #225
Fix MySQL commands #225
Conversation
frenck
added
the
bugfix
WalkthroughThe changes involve two main modifications. First, in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
phpmyadmin/rootfs/etc/s6-overlay/s6-rc.d/init-phpmyadmin/run (2)
27-31: LGTM! Consider documenting SSL security implications.The change from
mysqltomariadbcommand and addition of--skip-sslis correct and aligns with the PR objectives. However, disabling SSL removes transport layer security.Consider adding a comment explaining why SSL is disabled:
--skip-column-names \ + # SSL disabled as Home Assistant doesn't support it yet --skip-ssl \
37-41: Add error handling for database creation.The database creation command lacks error handling and success logging.
Consider wrapping the database creation in error handling:
bashio::log.info "Creating database for phpMyAdmin" + if ! \ mariadb \ -u "${username}" -p"${password}" \ -h "${host}" -P "${port}" \ --skip-ssl \ < /var/www/phpmyadmin/sql/create_tables.sql + then + bashio::log.error "Failed to create phpMyAdmin database" + exit 1 + fi + bashio::log.info "Successfully created phpMyAdmin database"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
.vscode/settings.json(1 hunks)phpmyadmin/rootfs/etc/s6-overlay/s6-rc.d/init-phpmyadmin/run(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .vscode/settings.json
| mariadb \ | ||
| -u "${username}" -p"${password}" \ | ||
| -h "${host}" -P "${port}" \ | ||
| --skip-ssl \ |
There was a problem hiding this comment.
💡 Codebase verification
Schema file location issue needs attention
The schema file /var/www/phpmyadmin/sql/create_tables.sql should be present in the phpMyAdmin package as it's part of the standard distribution, but it might have been removed during the Docker image build. Looking at the Dockerfile, there's a cleanup section that removes several directories but doesn't explicitly mention the sql directory:
&& rm -f -r \
/tmp/* \
/etc/nginx \
/var/www/phpmyadmin/setup \
/var/www/phpmyadmin/examples \
/var/www/phpmyadmin/test/ \
/var/www/phpmyadmin/po \
/var/www/phpmyadmin/composer.json \
/var/www/phpmyadmin/RELEASE-DATE-*The initialization script expects this file to exist but our verification shows it's missing. This needs to be fixed to ensure proper database initialization.
- Ensure the
sqldirectory and its contents are preserved during the Docker build process - Verify the schema file is present in phpMyAdmin 5.2.1 package before cleanup operations
🔗 Analysis chain
Verify schema file existence and accessibility.
The command changes look good, but let's verify the schema file exists and has correct permissions.
Let me search for the schema file in the repository to ensure we're looking at the correct path.
Let me try a different approach using ripgrep to search for any references to the schema file or SQL initialization in the codebase.
Let me check if the schema file is part of the phpMyAdmin installation package by looking at the Dockerfile or installation steps.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and permissions of the schema file
# Check if the file exists and show its permissions
ls -l /var/www/phpmyadmin/sql/create_tables.sql 2>/dev/null || echo "Schema file not found!"
# Check if the file is readable by the script's user
if [ -r "/var/www/phpmyadmin/sql/create_tables.sql" ]; then
echo "Schema file is readable"
else
echo "Schema file is not readable!"
fi
Length of output: 226
Script:
#!/bin/bash
# Search for create_tables.sql file in the repository
fd "create_tables.sql"
# Search for any SQL files that might contain table creation scripts
fd -e sql | grep -i "table"
Length of output: 50
Script:
#!/bin/bash
# Search for references to create_tables.sql or similar initialization files
rg -i "create.*tables\.sql"
# Look for SQL initialization or database setup references in the init script
rg -l "mariadb|mysql" phpmyadmin/rootfs/etc/s6-overlay/s6-rc.d/init-phpmyadmin/
# Check the content of the init script to understand the schema initialization
cat phpmyadmin/rootfs/etc/s6-overlay/s6-rc.d/init-phpmyadmin/run
Length of output: 1660
Script:
#!/bin/bash
# Check Dockerfile for phpMyAdmin installation
cat phpmyadmin/Dockerfile
# Look for any setup or installation scripts
fd -t f . phpmyadmin/rootfs/etc/s6-overlay/s6-rc.d/ -x cat {}
Length of output: 7917
Proposed Changes
mysqlis being replaced bymariadb(alias is now deprecated).Summary by CodeRabbit
New Features
Bug Fixes
Chores
--skip-ssloption to database commands for improved connection settings.