Skip to content

Edit file task.sh#169

Open
Dima-shima wants to merge 2 commits into
mate-academy:mainfrom
Dima-shima:main
Open

Edit file task.sh#169
Dima-shima wants to merge 2 commits into
mate-academy:mainfrom
Dima-shima:main

Conversation

@Dima-shima

Copy link
Copy Markdown

No description provided.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

I am requesting changes because the current task.sh still has a critical logic issue that can break the required workflow. The main blocker is the explicit CREATE DATABASE ShopDBReserve; in task.sh (around the section where you restore into ShopDBReserve): this conflicts with the requirement that databases are created via create-database.sql, and it will fail if ShopDBReserve already exists or if the backup user lacks global CREATE privileges, which violates the expected setup [CHECKLIST ITEM #2, #10, #12]. You should remove this CREATE DATABASE call and instead run the restore directly into the existing ShopDBReserve database, assuming it has been pre-created by create-database.sql.

The rest of the script generally follows the requirements: you correctly use DB_USER / DB_PASSWORD, run without arguments, and perform a data-only dump for ShopDBDevelopment. As a smaller improvement (not a blocker), consider adding a simple check at the top of the script to ensure DB_USER and DB_PASSWORD are set and perhaps clarifying comments distinguishing the full backup vs data-only backup steps. Once you adjust the ShopDBReserve handling to restore into the existing database, your solution will align with the functional requirements and be safe to use in the described environment. Keep going—you’re very close, and this fix will make your script robust and compliant with the task’s constraints.


✨ 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

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread task.sh Outdated
#! /bin/bash
mysqldump -u "$DB_USER" -p"$DB_PASSWORD" ShopDB \
--no-create-db --result-file=backup-no-create-db.sql
mysql -u "$DB_USER" -p"$DB_PASSWORD" -e "CREATE DATABASE ShopDBReserve;"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Calling CREATE DATABASE ShopDBReserve; here conflicts with the requirement that databases are created via create-database.sql and may fail if the DB already exists or the user lacks global CREATE privileges; consider restoring into an existing ShopDBReserve instead.

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review Summary

Your solution is approved because it now meets the core functional requirements of the task. The script correctly performs a full schema+data backup from ShopDB and restores it into ShopDBReserve using mysqldump, and it also creates a data-only dump (using --no-create-info) that is restored into ShopDBDevelopment, which ensures the development schema is not overwritten. All MySQL operations are consistently using the DB_USER and DB_PASSWORD environment variables, and the script runs without needing any command-line arguments, making it suitable for use in a cron-based nightly job.

One area you could improve (not required for this assignment) is adding a simple check at the top of task.sh to verify that DB_USER and DB_PASSWORD are set, so failures produce clearer error messages. Another optional enhancement would be to write backup files to a dedicated directory and/or include timestamps in the filenames to avoid accidental overwrites. Overall, you’ve implemented the required backup workflow correctly and aligned the script with the constraints that databases are pre-created via create-database.sql. Nice work bringing this in line with the specifications.


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants