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 email alerts on application server errors #42

Conversation

suecarmol
Copy link
Contributor

@suecarmol suecarmol commented Aug 22, 2020

Description

Added a setting in the production and staging environments that allow sending an email whenever there is an application error.

Rationale

This will help track any errors that may occur in the application.

Phabricator Ticket

T258144

How Has This Been Tested?

TBD

Screenshots of your changes (if appropriate):

N/A

Types of changes

What types of changes does your code introduce? Add an x in all the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

@suecarmol suecarmol mentioned this pull request Sep 4, 2020
3 tasks
@suecarmol suecarmol force-pushed the suecarmol/T258144-email-send-on-app-errors branch from 83681dc to a97e774 Compare September 4, 2020 23:24
@jsnshrmn
Copy link
Member

jsnshrmn commented Sep 9, 2020

@jsnshrmn jsnshrmn force-pushed the suecarmol/T258144-email-send-on-app-errors branch 2 times, most recently from ff59e51 to 559481c Compare September 9, 2020 19:20
@jsnshrmn
Copy link
Member

jsnshrmn commented Sep 9, 2020

okay, I've reworked our docker-related variable names and verified that this is pushing the correct images to dockerhub when I overwrite staging with it.
I did

docker system prune --all --force 
docker pull wikipedialibrary/eventstream:branch_staging
docker pull wikipedialibrary/externallinks:branch_staging
docker-compose up

and was able to get a working deployment without building anything.

I did find that we currently can have problems if we have workflow overruns between commits. I spun my wheels for a while trying to get an environment variable injected into a gh artifact name. I don't have this solved, but basically, we should be able to add a commit sha or action run id into the artifact name. It just doesn't seem to be working as I'd expect based on the documentation.

https://github.com/actions/upload-artifact#environment-variables-and-tilde-expansion
https://docs.github.com/en/actions/reference/context-and-expression-syntax-for-github-actions#env-context

@suecarmol suecarmol force-pushed the suecarmol/T258144-email-send-on-app-errors branch from 559481c to 174c7d4 Compare September 10, 2020 00:01
@suecarmol
Copy link
Contributor Author

I think I managed to inject a run_id into the artifact names. Is this what you meant? Let me know if I need to make any additional changes.

@jsnshrmn
Copy link
Member

Thanks! That's exactly what I was hoping to accomplish! Maybe the env context is only available to tasks running in a shell? This looks good from the github actions perspective!

@jsnshrmn jsnshrmn closed this Sep 11, 2020
@jsnshrmn jsnshrmn reopened this Sep 11, 2020
@jsnshrmn
Copy link
Member

Fat-fingered the close button!

@suecarmol
Copy link
Contributor Author

Before merging this to master, I would like to test this out on the staging environment. Otherwise, I think this PR is ready to merge

@jsnshrmn
Copy link
Member

Okay, I deployed a new staging environment with the current staging branch. I updated the project wiki notes as well.
https://github.com/WikipediaLibrary/externallinks/wiki/Debian-Server-Setup
Note that this will not currently pull new images automatically, as we talked about adding backups first.

Copy link
Member

@jsnshrmn jsnshrmn left a comment

Choose a reason for hiding this comment

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

Should use project and environment specific email addresses. See inline suggestions.

extlinks/settings/local.py Outdated Show resolved Hide resolved
extlinks/settings/production.py Outdated Show resolved Hide resolved
extlinks/settings/staging.py Outdated Show resolved Hide resolved
Copy link
Member

@jsnshrmn jsnshrmn left a comment

Choose a reason for hiding this comment

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

This is looking great! Thanks for your work!

@jsnshrmn jsnshrmn merged commit 744a911 into WikipediaLibrary:master Sep 16, 2020
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.

None yet

3 participants