You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/05/30 06:22:57 UTC
[GitHub] [airflow] satishmittal111 opened a new pull request #16166: Fixing SES backend - source (mail_from) must be string.
satishmittal111 opened a new pull request #16166:
URL: https://github.com/apache/airflow/pull/16166
The Source in send_raw_email must be a String type. mail_from=None is failing the SES backend for airflow alerts.
Currently SES Backend is failing with error -
```
File "/Users/username/Library/Python/3.8/lib/python/site-packages/airflow/models/taskinstance.py", line 1852, in email_alert
send_email(self.task.email, subject, html_content)
File "/Users/username/Library/Python/3.8/lib/python/site-packages/airflow/utils/email.py", line 52, in send_email
return backend(
File "/Users/username/Library/Python/3.8/lib/python/site-packages/airflow/providers/amazon/aws/utils/emailer.py", line 39, in send_email
hook.send_email(
File "/Users/username/Library/Python/3.8/lib/python/site-packages/airflow/providers/amazon/aws/hooks/ses.py", line 95, in send_email
return ses_client.send_raw_email(
File "/Users/username/Library/Python/3.8/lib/python/site-packages/botocore/client.py", line 357, in _api_call
return self._make_api_call(operation_name, kwargs)
......
File "/Users/username/Library/Python/3.8/lib/python/site-packages/botocore/validate.py", line 293, in serialize_to_request
raise ParamValidationError(report=report.generate_report())
botocore.exceptions.ParamValidationError: Parameter validation failed:
Invalid type for parameter Source, value: None, type: <class 'NoneType'>, valid types: <class 'str'>
```
Refer - https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/ses.html#SES.Client.send_raw_email
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] satishmittal111 commented on a change in pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
satishmittal111 commented on a change in pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#discussion_r642084421
##########
File path: airflow/providers/amazon/aws/utils/emailer.py
##########
@@ -36,8 +37,10 @@ def send_email(
) -> None:
"""Email backend for SES."""
hook = SESHook(aws_conn_id=conn_id)
+
+ mail_from = conf.get('smtp', 'SMTP_MAIL_FROM')
Review comment:
Thanks @mik-laj, I will update the Pr with tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-911391128
Since we’re very close to 2.2 now, anyone interested please feel free to open a separate PR inheriting works in here. I’m also adding the *pending-response* label so @satishmittal111 please respond if you are able to finish this soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ignaski commented on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
ignaski commented on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-911381634
This PR seems to be stuck :(
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-873394140
> @uranusjr currently we have smtp, sendgrid and aws SES as airflow backend. Sendgrid and smtp backend are using SENDGRID_MAIL_FROM and SMTP_MAIL_FROM environment variables respectively for "from" email. Should we create environment variable SES_MAIL_FROM for this (as per pattern)? cc - @mik-laj
While this is likely not perfect, I think it's better to make it consistent.
This is what sendgrid uses currently:
```
from_email = kwargs.get('from_email') or os.environ.get('SENDGRID_MAIL_FROM')
from_name = kwargs.get('from_name') or os.environ.get('SENDGRID_MAIL_SENDER')**
````
It allows for both kwargs and environment variables set, but with standard emails sent by airflow (alert/sla) the kwargs are not used anyway. However the documentation in https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/email-config.rst does not mention those env vars. It mentions few other '[email]' options but it does not mention any of the [smtp] options which are used by the standard "send_email_smtp" backend.
```
[smtp]
smtp_user = airflow
smtp_password = airflow
smtp_mail_from = airflow@example.com
```
So we have a bit of a mess to be honest. Proposal:
How about making it consistent and update the documentation at the same time between the three backends.
For all backends we could use:
SMTP:
```
from_email = kwargs.get('from_email') or os.environ.get('SMTP_MAIL_FROM') or conf.get('smtp', 'smtp_mail_from')
from_name = kwargs.get('from_name') or os.environ.get('SMTP_MAIL_SENDER') or conf.get('smtp', 'smtp_mail_sender')
```
SENDGRID:
```
from_email = kwargs.get('from_email') or os.environ.get('SENDGRID_MAIL_FROM') or conf.get('sendgrid', 'sendgrid_mail_from')
from_name = kwargs.get('from_name') or os.environ.get('SENDGRID_MAIL_SENDER') or conf.get('sendgrid', 'sendgrid_mail_sender')
```
SES:
```
from_email = kwargs.get('from_email') or os.environ.get('SES_MAIL_FROM') or conf.get('ses', 'ses_mail_from')
from_name = kwargs.get('from_name') or os.environ.get('SES_MAIL_SENDER') or conf.get('ses', 'ses_mail_sender')
```
And update the email documentation about it (also mentioning that you need the corresponding providers to use it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jzuhusky commented on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
jzuhusky commented on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-882853853
@potiuk @satishmittal111 @mik-laj
Any updates here? I just ran into this issue. Seems like the fix is done? Any idea when this might be released?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] satishmittal111 commented on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
satishmittal111 commented on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-877929428
@potiuk Sorry for the late response. The proposal looks good. Changes -
- Update documentation with Env Variables.
- Add Env Variables for SES backend.
I will be able to complete the PR by end of this week. Will it work for you?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ignaski edited a comment on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
ignaski edited a comment on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-911544284
> > This PR seems to be stuck :(
>
> Maybe you would like to take it over @ignaski ? Seems the solution is there, only some tests need to be added.
I will try to. I've never contributed to an open source project, thus it will take time to setup and get familiar.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-851001042
I think we should just require the user to pass in `email_from` manually instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ignaski commented on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
ignaski commented on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-911544284
> > This PR seems to be stuck :(
>
> Maybe you would like to take it over @ignaski ? Seems the solution is there, only some tests need to be added.
I will try to. I've never contributed to an open source project, thus it will take time to setup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-877863055
hello?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] satishmittal111 commented on a change in pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
satishmittal111 commented on a change in pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#discussion_r642031211
##########
File path: airflow/providers/amazon/aws/utils/emailer.py
##########
@@ -36,8 +37,10 @@ def send_email(
) -> None:
"""Email backend for SES."""
hook = SESHook(aws_conn_id=conn_id)
+
+ mail_from = conf.get('smtp', 'SMTP_MAIL_FROM')
Review comment:
Please guide if this is right approach or should we update the EmailOperator to have input for "from".
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-877618195
Hey @satishmittal111 - are you going to improve the PR ? I am preparing to release the next wave of providers and it would be great to get it merged before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-885840091
Not yet - seems that @satishmittal111 has to complete it yet :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] satishmittal111 edited a comment on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
satishmittal111 edited a comment on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-851009719
@uranusjr currently we have smtp, sendgrid and aws SES as airflow backend. Sendgrid and smtp backend are using SENDGRID_MAIL_FROM and SMTP_MAIL_FROM environment variables respectively for "from" email. Should we create environment variable SES_MAIL_FROM for this (as per pattern)? cc - @mik-laj
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-946272873
Continuing in #18042.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-946259427
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on a change in pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
mik-laj commented on a change in pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#discussion_r642077671
##########
File path: airflow/providers/amazon/aws/utils/emailer.py
##########
@@ -36,8 +37,10 @@ def send_email(
) -> None:
"""Email backend for SES."""
hook = SESHook(aws_conn_id=conn_id)
+
+ mail_from = conf.get('smtp', 'SMTP_MAIL_FROM')
Review comment:
It looks good. Can you add tests to avoid regression?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-911391114
> This PR seems to be stuck :(
Maybe you would like to take it over @ignaski ? Seems the solution is there, only some tests need to be added.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] satishmittal111 commented on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
satishmittal111 commented on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-911561868
Apologies. I was not able to take it further due to personal priority. Please take over the changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] satishmittal111 commented on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
satishmittal111 commented on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-851009719
@uranusjr currently we have smtp, sendgrid and aws SES as airflow backend. Sendgrid and smtp backend are using SENDGRID_MAIL_FROM and SMTP_MAIL_FROM environment variables respectively for "from" email. Should we create environment variable SES_MAIL_FROM for this (as per pattern)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jzuhusky commented on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
jzuhusky commented on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-882853853
@potiuk @satishmittal111 @mik-laj
Any updates here? I just ran into this issue. Seems like the fix is done? Any idea when this might be released?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-878019427
> I will be able to complete the PR by end of this week. Will it work for you?
Sure. I plan to release providers before, but I can also be released separately/next time :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on a change in pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#discussion_r701842885
##########
File path: airflow/providers/amazon/aws/utils/emailer.py
##########
@@ -36,8 +37,10 @@ def send_email(
) -> None:
"""Email backend for SES."""
hook = SESHook(aws_conn_id=conn_id)
+
+ mail_from = conf.get('smtp', 'SMTP_MAIL_FROM')
Review comment:
@satishmittal111 Are you able to update this PR with tests?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jzuhusky commented on pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
jzuhusky commented on pull request #16166:
URL: https://github.com/apache/airflow/pull/16166#issuecomment-882853853
@potiuk @satishmittal111 @mik-laj
Any updates here? I just ran into this issue. Seems like the fix is done? Any idea when this might be released?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr closed pull request #16166: Fixing SES backend - source (mail_from) must be string.
Posted by GitBox <gi...@apache.org>.
uranusjr closed pull request #16166:
URL: https://github.com/apache/airflow/pull/16166
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org