Done#171
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because the script currently fails several core functional requirements and the tests are breaking as a result. The mysqldump commands in task.sh are using the literal strings DB_USER and DB_PASSWORD instead of the environment variables $DB_USER and $DB_PASSWORD, which causes authentication errors and prevents any backup from being created [CHECKLIST ITEM #2, #12]. The restore step to ShopDBReserve incorrectly uses mysqldump with input redirection instead of the mysql client (mysql -u "$DB_USER" -p"$DB_PASSWORD" ShopDBReserve < ShopDB-Full.sql), so the full backup is not properly imported [CHECKLIST ITEM #1–3, #12]. For the development database, the script copies /var/lib/mysql/ShopDB directly to /var/lib/mysql/ShopDBDevelopment, which bypasses the required mysqldump/mysql workflow, copies schema/internal files from production, and does not implement a data-only SQL backup/restore as specified [CHECKLIST ITEM #1–3, #12]. These issues mean the Products table is not guaranteed to exist with matching row counts in ShopDB, ShopDBReserve, and ShopDBDevelopment, which is a critical requirement [CHECKLIST ITEM #3].
To fix this, you should: 1) use $DB_USER and $DB_PASSWORD in all MySQL commands; 2) perform a standard full dump from ShopDB with mysqldump and restore it into ShopDBReserve using mysql; and 3) create a data-only dump from ShopDB (e.g., with --no-create-info) and restore it into ShopDBDevelopment, assuming its schema is pre-created by the provided SQL scripts. As minor improvements, consider adding simple error checks on each command so cron can detect failures and optionally log basic status messages, but these are secondary to fixing the backup/restore logic. Once you adjust the script to follow the SQL-based backup/restore flow and use environment variables correctly, it should satisfy the requirements and pass the tests. Keep going—your overall structure is close; it mainly needs the correct tools and options wired up as described.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| #! /bin/bash | ||
|
|
||
| # Create full backup the database ShopDB | ||
| mysqldump -u DB_USER -pDB_PASSWORD --databases ShopDB --result-file=ShopDB-Full.sql --no-create-db |
There was a problem hiding this comment.
This mysqldump call is using literal DB_USER and DB_PASSWORD instead of the environment variables; it should reference $DB_USER and $DB_PASSWORD to satisfy the requirement of reading credentials from env vars.
| # Create full backup the database ShopDB | ||
| mysqldump -u DB_USER -pDB_PASSWORD --databases ShopDB --result-file=ShopDB-Full.sql --no-create-db | ||
| # Restore full backup the database ShopDB in the database ShopDBReserve | ||
| mysqldump -u DB_USER -pDB_PASSWORD ShopDBReserve < ShopDB-Full.sql |
There was a problem hiding this comment.
To restore a backup, you should use the mysql client to import ShopDB-Full.sql into ShopDBReserve (e.g., mysql -u "$DB_USER" -p"$DB_PASSWORD" ShopDBReserve < ShopDB-Full.sql), not mysqldump with input redirection.
| if [ -d "/var/lib/mysql/ShopDB" ]; then | ||
| # Copy with overwrite (-r for recursive, -f for force overwrite) | ||
| cp -r /var/lib/mysql/ShopDB /var/lib/mysql/ShopDBDevelopment |
There was a problem hiding this comment.
Copying the raw database directory directly from /var/lib/mysql/ShopDB to /var/lib/mysql/ShopDBDevelopment does not meet the requirement to create a data backup using mysqldump and restore it; it also copies schema and internal files, which conflicts with the requirement that schema should not be copied from production to development.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I am requesting changes because one core requirement of the task is still not met and this directly affects the backup/restore workflow. The specification explicitly requires a two-step full backup restore: first create a dump file from ShopDB (e.g., mysqldump ... ShopDB > ShopDB-Full.sql), then restore that file into ShopDBReserve using mysql with input redirection (e.g., mysql ... ShopDBReserve < ShopDB-Full.sql). In your current task.sh, the restore to ShopDBReserve is done via a mysqldump | mysql pipeline, which violates that requirement and [CHECKLIST ITEM] about using the dump-file-plus-mysql workflow.
To fix this, adjust the full backup section so that it writes the dump to a file, and then add a separate command that uses mysql to restore from that file into ShopDBReserve. For example, near the top of the script define a filename like FULL_DUMP="ShopDB-Full.sql", then use mysqldump -u "$DB_USER" -p"$DB_PASSWORD" ShopDB > "$FULL_DUMP" followed by mysql -u "$DB_USER" -p"$DB_PASSWORD" ShopDBReserve < "$FULL_DUMP". Your environment variable usage, argument handling (none required), and the data-only backup/restore to ShopDBDevelopment all look good and align with the task.
Once you update the full backup restore to use the required two-step file-based workflow, your script should fully comply with the task description. You’re very close—this is a good implementation overall, and fixing this restore method will complete the assignment correctly.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| #!/bin/bash | ||
|
|
||
| # Full backup of ShopDB (schema + data), restore to ShopDBReserve | ||
| mysqldump -u "$DB_USER" -p"$DB_PASSWORD" ShopDB | mysql -u "$DB_USER" -p"$DB_PASSWORD" ShopDBReserve |
There was a problem hiding this comment.
The restore to ShopDBReserve is done via a mysqldump | mysql pipeline, but the requirements and prior high-severity correction specify creating a full dump file and restoring it with mysql using input redirection (e.g., mysqldump ... ShopDB > ShopDB-Full.sql then mysql ... ShopDBReserve < ShopDB-Full.sql). Please adjust to that two-step workflow to match the task specification.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved—task.sh now correctly implements the required backup and restore workflow using mysqldump and mysql, and it reads credentials from the DB_USER and DB_PASSWORD environment variables as specified. The full backup of ShopDB to a dump file and separate restore into ShopDBReserve matches the two-step file-based process the task called for, and the data-only backup to ShopDBDevelopment (using --no-create-info) correctly avoids copying the schema while ensuring Products data is synchronized.
You did well ensuring the script runs non-interactively with no arguments, making it suitable for cron and automated execution. One minor improvement for robustness would be to add checks near the top of the script to verify that DB_USER and DB_PASSWORD are set, and print a clear error message if they are missing (for example, checking [ -z "$DB_USER" ] and exiting with a helpful note). Overall, this is a solid implementation that meets the functional and technical requirements, and it shows good attention to the backup/restore semantics and environment-variable usage. Nicely done—this is strong progress and a good pattern to build on for future automation scripts.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
3 participants
No description provided.