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

Adapt SendGrid integration to use the updated libraries. #2715

Merged
merged 6 commits into from
May 28, 2019

Conversation

guidopetri
Copy link
Contributor

Description

SendGrid changed their API about three years ago to use API tokens instead of username/password combos. The sendgrid library adapted, but Luigi's code that depends on SendGrid did not.

Motivation and Context

This re-instates SendGrid integration for sending out emails when Luigi Tasks fail. It solves #2714 and #1956 (since they are duplicate issues).

Have you tested this? If so, how?

I ran a simple Task that force-failed by calling sys.exit(), together with a config file using an apikey value in [sendgrid] and a method=sendgrid on [email]. I received the "Task failed" emails, even with multiple email addresses set up.

luigi/notifications.py Outdated Show resolved Hide resolved
luigi/notifications.py Outdated Show resolved Hide resolved
@dlstadther
Copy link
Collaborator

Please take a look at failed tests. Looks like you'll need to fix the flake8 and update the test cfg for apikey

@guidopetri
Copy link
Contributor Author

jeez, that took way too long for me to understand what was going on.

tests should all pass now. sorry for the back-and-forth - i'm not super familiar with travis/tox and unit testing in general. does this look good or should any more changes be done?

Copy link
Collaborator

@dlstadther dlstadther left a comment

Choose a reason for hiding this comment

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

No worries! LGTM

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