dagster icon indicating copy to clipboard operation
dagster copied to clipboard

smtp auth user vs email_from

Open chunkerchunker opened this issue 1 year ago • 4 comments

Summary & Motivation

Some email servers, such as AWS SES, require a separate authentication user from the email from address. But the make_email_on_run_failure_sensor() utility requires them to be the same.

This PR is a small change to allow specifying them separately. Backwards compatibility with previous behavior is maintained by defaulting the auth user to the email from address.

This PR also incorporates the fix from PR #16585 to return the sensor skip reason.

How I Tested These Changes

I tested the change with the latest dagster release, sending emails from both AWS SES (which requires separate auth user vs email from) and Gmail (which doesn't).

chunkerchunker avatar Jan 05 '24 09:01 chunkerchunker

@yuhan noticed that these were functionalities you added a few years ago, would you be up to review this?

clairelin135 avatar Jan 05 '24 18:01 clairelin135

Apologies, I've fixed the ruff format error that caused the failure below.

chunkerchunker avatar Jan 05 '24 21:01 chunkerchunker

Really useful functionality. looking forward to implementation.

kira2600 avatar Feb 15 '24 13:02 kira2600

Hello, any options that it will be implemented? Thanks.

kira2600 avatar May 17 '24 09:05 kira2600

While handling the merge conflict, I realized a similar commit was merged a few days ago: https://github.com/dagster-io/dagster/commit/774d1d3ded5263b95f201ba5f239fb66f51bc9d5. Therefore, I’ll be closing this.

Thank you for your effort in submitting this contribution, and I apologize for any inconvenience caused by the timing. This fell through the cracks because I lost track of this open PR.

yuhan avatar May 24 '24 08:05 yuhan