Skip to content

Solution#159

Open
nichelangeloo wants to merge 2 commits into
mate-academy:mainfrom
nichelangeloo:develop
Open

Solution#159
nichelangeloo wants to merge 2 commits into
mate-academy:mainfrom
nichelangeloo:develop

Conversation

@nichelangeloo

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

The script has critical blockers that prevent it from meeting the task requirements. 1. Wrong credential source: The script assigns DB_USER=$1 and DB_PASSWORD=$2 from positional arguments instead of reading from environment variables. Per the requirements, credentials must come from the DB_USER and DB_PASSWORD environment variables. This makes the script unable to run without arguments, which violates the specification. 2. Missing password authentication: All mysqldump and mysql commands lack the password option. The check run confirms this with "using password: NO", causing authentication failures. You need to add -p"$DB_PASSWORD" to each MySQL command, or use the MYSQL_PWD environment variable approach. Fixes needed: Replace the credential assignments at the top of the script to read from environment variables, and ensure each database command includes the password. The mysqldump commands need -u "$DB_USER" -p"$DB_PASSWORD" and the mysql restore commands need the same authentication flags. The script structure and logic (using --no-create-info for data-only backup, correct database names) are correct—only the credential handling needs to be corrected.


✨ 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
@@ -1 +1,7 @@
#! /bin/bash

DB_USER=$1

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 script reads DB_USER from $1 (command-line argument) instead of from the environment variable DB_USER. Per the requirements, this should read from the environment variable.

Comment thread task.sh Outdated
#! /bin/bash

DB_USER=$1
export DB_PASSWORD=$2

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 script reads DB_PASSWORD from $2 (command-line argument) instead of from the environment variable DB_PASSWORD. Per the requirements, this should read from the environment variable.

Comment thread task.sh Outdated
DB_USER=$1
export DB_PASSWORD=$2

mysqldump -u "$DB_USER" ShopDB | mysql -u "$DB_USER" 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.

The mysqldump command is missing the password option. Since the script should use the credentials from environment variables, you need to include -p"$DB_PASSWORD" for authentication.

Comment thread task.sh Outdated
DB_USER=$1
export DB_PASSWORD=$2

mysqldump -u "$DB_USER" ShopDB | mysql -u "$DB_USER" 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.

The mysql command for restoring to ShopDBReserve is missing the password option. You need to include -p"$DB_PASSWORD" for authentication.

Comment thread task.sh Outdated
export DB_PASSWORD=$2

mysqldump -u "$DB_USER" ShopDB | mysql -u "$DB_USER" ShopDBReserve
mysqldump -u "$DB_USER" --no-create-info ShopDB | mysql -u "$DB_USER" ShopDBDevelopment No newline at end of file

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 mysqldump command for the data backup is missing the password option. You need to include -p"$DB_PASSWORD" for authentication.

Comment thread task.sh Outdated
export DB_PASSWORD=$2

mysqldump -u "$DB_USER" ShopDB | mysql -u "$DB_USER" ShopDBReserve
mysqldump -u "$DB_USER" --no-create-info ShopDB | mysql -u "$DB_USER" ShopDBDevelopment No newline at end of file

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 mysql command for restoring to ShopDBDevelopment is missing the password option. You need to include -p"$DB_PASSWORD" for authentication.

@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

The script correctly implements all task requirements: it reads database credentials from DB_USER and DB_PASSWORD environment variables, performs a full backup of ShopDB and restores it to ShopDBReserve, creates a data-only backup using --no-create-info flag, and restores it to ShopDBDevelopment. All database commands include proper authentication flags with -u "$DB_USER" -p"$DB_PASSWORD", and the script runs without requiring any arguments. Great work on fixing the credential handling issues—your implementation is now complete and ready for use.


✨ 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