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

feat: SMTP as a fallback function when Sendgrid quota exceeds limit #5981

Merged
merged 1 commit into from
Jun 20, 2019

Conversation

mrsaicharan1
Copy link
Member

Fixes #5958

Checklist

  • I have read the Contribution & Best practices Guide and my PR follows them.
  • My branch is up-to-date with the Upstream development branch.
  • The unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • All the functions created/modified in this PR contain relevant docstrings.

Short description of what this resolves:

  • creates a centralized task rather than having two separate tasks for them
  • Checks for Sendgrid quota value and decides the protocol

Changes proposed in this pull request:

  • Added centralised task send_email_via_smtp
  • Add check for SMTP fallback

@mrsaicharan1 mrsaicharan1 changed the title [WIP]feat: creating SMTP as a fallback function when Sendgrid quota exceeds feat: creating SMTP as a fallback function when Sendgrid quota exceeds Jun 1, 2019
@auto-label auto-label bot added the feature label Jun 1, 2019
@mrsaicharan1 mrsaicharan1 changed the title feat: creating SMTP as a fallback function when Sendgrid quota exceeds feat: creating SMTP as a fallback function when Sendgrid quota exceeds limit Jun 1, 2019
@fossasia fossasia deleted a comment Jun 1, 2019
@fossasia fossasia deleted a comment Jun 1, 2019
@fossasia fossasia deleted a comment Jun 1, 2019
@mrsaicharan1 mrsaicharan1 changed the title feat: creating SMTP as a fallback function when Sendgrid quota exceeds limit [wip]feat: creating SMTP as a fallback function when Sendgrid quota exceeds limit Jun 1, 2019
@auto-label auto-label bot removed the feature label Jun 1, 2019
@fossasia fossasia deleted a comment Jun 3, 2019
@fossasia fossasia deleted a comment Jun 3, 2019
@fossasia fossasia deleted a comment Jun 3, 2019
@codecov
Copy link

codecov bot commented Jun 3, 2019

Codecov Report

❗ No coverage uploaded for pull request base (development@47363b9). Click here to learn what that means.
The diff coverage is 7.54%.

Impacted file tree graph

@@              Coverage Diff              @@
##             development   #5981   +/-   ##
=============================================
  Coverage               ?   66.1%           
=============================================
  Files                  ?     285           
  Lines                  ?   14122           
  Branches               ?       0           
=============================================
  Hits                   ?    9335           
  Misses                 ?    4787           
  Partials               ?       0
Impacted Files Coverage Δ
app/api/helpers/mail.py 29.29% <7.14%> (ø)
app/api/helpers/tasks.py 17.58% <8%> (ø)

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 47363b9...6d9f6d2. Read the comment docs.

@fossasia fossasia deleted a comment Jun 3, 2019
@fossasia fossasia deleted a comment Jun 3, 2019
@fossasia fossasia deleted a comment Jun 3, 2019
@fossasia fossasia deleted a comment Jun 3, 2019
@fossasia fossasia deleted a comment Jun 3, 2019
@fossasia fossasia deleted a comment Jun 3, 2019
@fossasia fossasia deleted a comment Jun 3, 2019
@fossasia fossasia deleted a comment Jun 3, 2019
@fossasia fossasia deleted a comment Jun 3, 2019
@fossasia fossasia deleted a comment Jun 3, 2019
@fossasia fossasia deleted a comment Jun 3, 2019
@fossasia fossasia deleted a comment Jun 3, 2019
@iamareebjamal
Copy link
Member

@mrsaicharan1

Tests are failing because your code is trying to send emails and failing:

 File "/home/travis/build/fossasia/open-event-server/app/api/auth.py", line 225, in reset_password_post
    send_email_with_action(user, PASSWORD_RESET, app_name=get_settings()['app_name'], link=link)
  File "/home/travis/build/fossasia/open-event-server/app/api/helpers/mail.py", line 119, in send_email_with_action
    html=MAILS[action]['message'].format(**kwargs)
  File "/home/travis/build/fossasia/open-event-server/app/api/helpers/mail.py", line 87, in send_email
    send_email_task_sendgrid.delay(payload=payload, headers=headers)
...
File "/home/travis/build/fossasia/open-event-server/app/api/helpers/tasks.py", line 76, in send_email_task_sendgrid
    sendgrid_client.send(message)
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/sendgrid/sendgrid.py", line 98, in send
    response = self.client.mail.send.post(request_body=message.get())
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/python_http_client/client.py", line 252, in http_request
    return Response(self._make_request(opener, request, timeout=timeout))
  File "/home/travis/virtualenv/python3.6.3/lib/python3.6/site-packages/python_http_client/client.py", line 176, in _make_request
    raise exc
python_http_client.exceptions.UnauthorizedError: HTTP Error 401: Unauthorized

@iamareebjamal
Copy link
Member

iamareebjamal commented Jun 19, 2019

Found the freaking culprit: https://api.eventyay.com/#settings-settings-details-patch

Not only it sets the sendgrid key to a random value, it also changes the environment to production!!!

Fix the docs and the error will be gone. Just change the app-environment to testing

@fossasia fossasia deleted a comment Jun 19, 2019
@iamareebjamal
Copy link
Member

Something else is changing the app config. Need to debug more

@mrsaicharan1
Copy link
Member Author

@iamareebjamal This is only due to the random app config change right? Because I made sure that the email is not being sent when neither of them are configured.

@iamareebjamal
Copy link
Member

Yes, it is due to config changes

@fossasia fossasia deleted a comment Jun 20, 2019
@fossasia fossasia deleted a comment Jun 20, 2019
@mrsaicharan1
Copy link
Member Author

@iamareebjamal Looks good I guess. Travis seems to go through now :)

uds5501
uds5501 previously approved these changes Jun 20, 2019
@mrsaicharan1 mrsaicharan1 force-pushed the smtp-fallback branch 2 times, most recently from 72f4e00 to 2a69f30 Compare June 20, 2019 11:12
@mrsaicharan1 mrsaicharan1 changed the title feat: SMTP as a fallback function when Sendgrid quota exceeds limit [WIP]feat: SMTP as a fallback function when Sendgrid quota exceeds limit Jun 20, 2019
@auto-label auto-label bot removed the feature label Jun 20, 2019
@mrsaicharan1 mrsaicharan1 changed the title [WIP]feat: SMTP as a fallback function when Sendgrid quota exceeds limit feat: SMTP as a fallback function when Sendgrid quota exceeds limit Jun 20, 2019
@auto-label auto-label bot added the feature label Jun 20, 2019
@mrsaicharan1
Copy link
Member Author

@iamareebjamal Please have a look now.

Modified arguments placement

Corrected quota info

Sent email and handled error

Cease email service if SMTP not configured

fix travis

divided into two tasks

changed logger message and smtp fallback
@iamareebjamal iamareebjamal merged commit c10a3fc into fossasia:development Jun 20, 2019
iamareebjamal pushed a commit to iamareebjamal/open-event-server that referenced this pull request Aug 2, 2019
…ossasia#5981)

Modified arguments placement

Corrected quota info

Sent email and handled error

Cease email service if SMTP not configured

fix travis

divided into two tasks

changed logger message and smtp fallback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FallBack to SMTP when SendGrid Fails/Exceeds Quota
5 participants