Skip to content
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 sending slack notification on deploy #75

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kdowolski-reef
Copy link

No description provided.

@@ -27,6 +27,11 @@ docker-compose run --rm app sh -c "python manage.py wait_for_database --timeout
# start everything
docker-compose up -d

{% if cookiecutter.use_slack_notification %}
# Send slack notification about deploy
docker-compose run --rm app sh -c "python bin/notify.py"
Copy link
Author

Choose a reason for hiding this comment

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

Not sure how working with that template looks like in real but deploy-to-aws (which is called by github action) doesn't call deploy (or I don't see something). If so, the problem where to call it stands.

Copy link
Contributor

Choose a reason for hiding this comment

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

What solution do you suggest?

Copy link
Author

Choose a reason for hiding this comment

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

I see two options.

  1. Run it here. Technically we have container built there so could run script. Currently we do not COPY bin directory to container but I guess it's not intended.
  2. Technically we could add it here but in case of restarting container we will get notifications.

@@ -27,6 +27,11 @@ docker-compose run --rm app sh -c "python manage.py wait_for_database --timeout
# start everything
docker-compose up -d

{% if cookiecutter.use_slack_notification %}
# Send slack notification about deploy
docker-compose run --rm app sh -c "python bin/notify.py"
Copy link
Contributor

Choose a reason for hiding this comment

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

What solution do you suggest?

@kdowolski-reef kdowolski-reef force-pushed the master branch 7 times, most recently from f44ef61 to 3ff6493 Compare June 19, 2022 10:17
@@ -29,7 +29,8 @@ docker-compose up -d

{% if cookiecutter.use_slack_notification %}
# Send slack notification about deploy
docker-compose run --rm app sh -c "python bin/notify.py"
output=`git log --format=format:%H,%s`
docker-compose run --rm app sh -c "python /root/src/bin/notify.py --parse -m \"$output\""
Copy link
Author

Choose a reason for hiding this comment

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

Unfortunately it allows for code injection with backticks, searching how to escape it properly.

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