Skip to content

Solved!#167

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

Solved!#167
ProstoPonchik wants to merge 2 commits into
mate-academy:mainfrom
ProstoPonchik:main

Conversation

@ProstoPonchik

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 there is a critical typo in the reserve database name that breaks the core functionality. In task.sh, the restore target is ShopDBReverse, but the requirements and the SQL setup use ShopDBReserve, so the full backup is not being restored into the correct reserve database. This directly violates the requirement that ShopDBReserve hold a restorable copy of ShopDB with the Products table and matching record counts ([CHECKLIST ITEM #2], [CHECKLIST ITEM #4], [CHECKLIST ITEM #12], [CHECKLIST ITEM #19]).

To fix this, update every occurrence of the mis-typed database name in task.sh so that any mysql or mysqldump commands target ShopDBReserve exactly (case and spelling must match the database created by create-database.sql). After that correction, re-run your script to ensure the backup to ShopDBReserve completes successfully and the tests that compare the Products table between ShopDB and ShopDBReserve will pass. Aside from this, your use of environment variables, non-interactive behavior, and --no-create-info for the development database looks solid and is aligned with the cron-friendly requirements. Once you correct the database name, your solution should fully meet the task’s expectations.


✨ 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
fi

mysqldump -u "$DB_USER" -p"$DB_PASSWORD" ShopDB --result-file=ShopDB.sql
mysql -u "$DB_USER" -p"$DB_PASSWORD" ShopDBReverse < ShopDB.sql

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The restore target is ShopDBReverse, but the requirements and the SQL grants use ShopDBReserve. This typo breaks checklist items #2, #12, and #4 because the backup is not restored into the correct reserve database.

@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 approving your solution because the task.sh script now cleanly meets the functional requirements: it performs a full ShopDB backup into ShopDBReserve and a data-only backup from ShopDB into ShopDBDevelopment using mysqldump and mysql, and it runs non-interactively using DB_USER and DB_PASSWORD environment variables, making it suitable for a cron job. The previous critical issue with the misspelled ShopDBReserve database name has been fully corrected everywhere, so the backups and restores will now target the intended databases.

Your implementation ensures that, once the databases and user are prepared via the separate SQL script and manual steps, tables like Products will end up in both ShopDBReserve and ShopDBDevelopment with matching record counts. There are no remaining blockers or missing features relative to the described task. As a minor, non-blocking improvement, you could add simple error handling (for example set -e at the top of the script or checking the exit codes of mysqldump/mysql) to make failures more visible in cron logs. Overall, this is a solid, task-focused script that aligns well with the stated requirements.


✨ 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