-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add configuration of mongodb and rabbotmq hosts #255
Conversation
Reviewer's Guide by SourceryThis pull request adds the configuration of MongoDB and RabbitMQ hosts to the File-Level Changes
Tips
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @lvarin - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
update-config-map.sh
Outdated
APP_CONFIG=$(yq --arg MONGO_HOST "$MONGO_HOST" \ | ||
--arg RABBIT_HOST "$RABBIT_HOST" \ | ||
'.db.host = $MONGO_HOST | | ||
.jobs.host = $RABBIT_HOST' \ | ||
"$APP_CONFIG_PATH") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Check for yq command availability.
Ensure that the script checks for the availability of the yq command before attempting to use it. This can prevent runtime errors if yq is not installed or not in the PATH.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks for adding the check for the yq
command availability! This will help prevent any runtime issues. If you encounter any further issues or need additional feedback, feel free to ask. Keep up the good work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
IMPORTANT: Please create an issue before filing a pull request! Changes need to be discussed before proceeding. Please read the contribution guidelines.
Details
Please provide enough information so that others can review your pull request. Give a brief summary of the motivation. Refer to the corresponding issue/s with
#XXXX
for more information.Testing
Write the appropriate unit and integration tests, if applicable. Make sure these and all other tests pass.
Documentation
Please document your changes and test cases in the appropriate places, if applicable.
Style
Make sure your changes adhere to the coding/documentation style used throughout the project.
Closing issues
If your changes fix any issue/s, put
closes #XXXX
in your comment to auto-close it/them.Credit
Add your credentials to the list of contributors once your pull request was merged.
Summary by Sourcery
Added configuration options for MongoDB and RabbitMQ hosts in the update-config-map.sh script. Default values are set to 'mongodb' and 'rabbitmq' respectively if not provided.