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

Notify via email of message transmission errors #44

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

Conversation

kenyonit
Copy link
Contributor

Send an email to managers for any message transmission errors

kenyonit added 9 commits June 29, 2020 12:39
Added function to notify of errors
Notify of failure to send
Notify of error on message receive
Notify of errors processing message
Fixed missing ')'
fixed blank line contains whitespace
Fixed code formatting
@codecov
Copy link

codecov bot commented Jun 29, 2020

Codecov Report

Merging #44 into master will decrease coverage by 0.44%.
The diff coverage is 80.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #44      +/-   ##
==========================================
- Coverage   97.56%   97.12%   -0.45%     
==========================================
  Files          11       11              
  Lines         576      591      +15     
==========================================
+ Hits          562      574      +12     
- Misses         14       17       +3     
Impacted Files Coverage Δ
pyas2/views.py 97.20% <75.00%> (-0.64%) ⬇️
pyas2/utils.py 93.10% <77.77%> (-6.90%) ⬇️
pyas2/models.py 96.09% <100.00%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2ad2183...3fbbc9e. Read the comment docs.

@chadgates
Copy link
Contributor

@kenyonit @abhishek-ram I generally would subscribe to the idea, that an email could be sent on failed message sending. From quick look at this code, I do not see to whom this should be sent. I suppose the email as defined in the Organization? Any thoughts on this @abhishek-ram ?

@zalejus
Copy link

zalejus commented Nov 29, 2023

Hi @abhishek-ram,
could you, please, tell us if you plan merging this parts of code into your project?
This is a very useful functionality that allows you to inform administrators that some messages have the ERROR status.
It would be great to improve your super solution. Believe me - many people use it :)

@zalejus
Copy link

zalejus commented Mar 28, 2024

@abhishek-ram,
are you still there? Are you planning to do something with this project or is it already dead? I mean above improvements.

@abhishek-ram
Copy link
Owner

Should we do logger error in all these places so that devs can make use of https://docs.djangoproject.com/en/5.0/ref/logging/#django.utils.log.AdminEmailHandler to send emails?

@zalejus
Copy link

zalejus commented Mar 29, 2024

@abhishek-ram No we shouldn't of course. I didn't know what you were writing about. You know, I'm not django expert. So many thank for your tips

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.

4 participants