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 2022/12/28 21:07:25 UTC
[GitHub] [airflow] stamixthereal opened a new pull request, #28634: Add doc-strings and small improvement to email util
stamixthereal opened a new pull request, #28634:
URL: https://github.com/apache/airflow/pull/28634
Docstrings were added to email sending util, some type hints changes, and implements regular expression intro `_get_email_list_from_str` function
--
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 a diff in pull request #28634: Add doc-strings and small improvement to email util
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28634:
URL: https://github.com/apache/airflow/pull/28634#discussion_r1058742159
##########
airflow/utils/email.py:
##########
@@ -47,8 +48,26 @@ def send_email(
conn_id: str | None = None,
custom_headers: dict[str, Any] | None = None,
**kwargs,
-):
- """Send email using backend specified in EMAIL_BACKEND."""
+) -> None:
+ """
+ Send an email using the backend specified in the `EMAIL_BACKEND` configuration option.
Review Comment:
```suggestion
Send an email using the backend specified in the ``EMAIL_BACKEND`` configuration option.
```
--
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 a diff in pull request #28634: Add doc-strings and small improvement to email util
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28634:
URL: https://github.com/apache/airflow/pull/28634#discussion_r1058742360
##########
airflow/utils/email.py:
##########
@@ -47,8 +48,26 @@ def send_email(
conn_id: str | None = None,
custom_headers: dict[str, Any] | None = None,
**kwargs,
-):
- """Send email using backend specified in EMAIL_BACKEND."""
+) -> None:
+ """
+ Send an email using the backend specified in the `EMAIL_BACKEND` configuration option.
+
+ :param to: A list or iterable of email addresses to send the email to.
+ :param subject: The subject of the email.
+ :param html_content: The content of the email in HTML format.
+ :param files: A list of paths to files to attach to the email.
+ :param dryrun: If `True`, the email will not actually be sent. Default: `False`.
Review Comment:
```suggestion
:param dryrun: If `True`, the email will not actually be sent. Default: `False`.
```
Either using double backticks or star (italic, matching the style used by the Python stdlib documentation)
--
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] stamixthereal commented on a diff in pull request #28634: Add doc-strings and small improvement to email util
Posted by GitBox <gi...@apache.org>.
stamixthereal commented on code in PR #28634:
URL: https://github.com/apache/airflow/pull/28634#discussion_r1058870872
##########
airflow/utils/email.py:
##########
@@ -87,7 +106,7 @@ def send_email_smtp(
from_email: str | None = None,
custom_headers: dict[str, Any] | None = None,
**kwargs,
-):
+) -> None:
Review Comment:
Yes, you are right, @potiuk!
--
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 a diff in pull request #28634: Add doc-strings and small improvement to email util
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28634:
URL: https://github.com/apache/airflow/pull/28634#discussion_r1058742434
##########
airflow/utils/email.py:
##########
@@ -47,8 +48,26 @@ def send_email(
conn_id: str | None = None,
custom_headers: dict[str, Any] | None = None,
**kwargs,
-):
- """Send email using backend specified in EMAIL_BACKEND."""
+) -> None:
+ """
+ Send an email using the backend specified in the `EMAIL_BACKEND` configuration option.
+
+ :param to: A list or iterable of email addresses to send the email to.
+ :param subject: The subject of the email.
+ :param html_content: The content of the email in HTML format.
+ :param files: A list of paths to files to attach to the email.
+ :param dryrun: If `True`, the email will not actually be sent. Default: `False`.
+ :param cc: A string or iterable of strings containing email addresses to send a copy of the email to.
+ :param bcc: A string or iterable of strings containing email addresses to send a
+ blind carbon copy of the email to.
+ :param mime_subtype: The subtype of the MIME message. Default: "mixed".
+ :param mime_charset: The charset of the email. Default: "utf-8".
+ :param conn_id: The connection ID to use for the backend. If not provided, the default connection
+ specified in the `EMAIL_CONN_ID` configuration option will be used.
Review Comment:
```suggestion
specified in the ``EMAIL_CONN_ID`` configuration option will be used.
```
--
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 a diff in pull request #28634: Add doc-strings and small improvement to email util
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #28634:
URL: https://github.com/apache/airflow/pull/28634#discussion_r1058832965
##########
airflow/utils/email.py:
##########
@@ -87,7 +106,7 @@ def send_email_smtp(
from_email: str | None = None,
custom_headers: dict[str, Any] | None = None,
**kwargs,
-):
+) -> None:
Review Comment:
Maybe a good idea to add parameter docstring here for consistency? Seems to be the last undocumented method here?
--
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] stamixthereal commented on pull request #28634: Add doc-strings and small improvement to email util
Posted by GitBox <gi...@apache.org>.
stamixthereal commented on PR #28634:
URL: https://github.com/apache/airflow/pull/28634#issuecomment-1367225862
Thanks for your review, @potiuk; I fixed 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] potiuk merged pull request #28634: Add doc-strings and small improvement to email util
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #28634:
URL: https://github.com/apache/airflow/pull/28634
--
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 a diff in pull request #28634: Add doc-strings and small improvement to email util
Posted by GitBox <gi...@apache.org>.
uranusjr commented on code in PR #28634:
URL: https://github.com/apache/airflow/pull/28634#discussion_r1058743022
##########
airflow/utils/email.py:
##########
@@ -267,8 +306,13 @@ def _get_smtp_connection(host: str, port: int, timeout: int, with_ssl: bool) ->
def _get_email_list_from_str(addresses: str) -> list[str]:
- delimiters = [",", ";"]
- for delimiter in delimiters:
- if delimiter in addresses:
- return [address.strip() for address in addresses.split(delimiter)]
- return [addresses]
+ """
+ Extract a list of email addresses from a string. The string
+ can contain multiple email addresses separated by
+ any of the following delimiters: ',' or ';'.
+
+ :param addresses: A string containing one or more email addresses.
+ :return: A list of email addresses.
+ """
+ pattern = r"[,;]\s*"
+ return [address.strip() for address in re.split(pattern, addresses)]
Review Comment:
Since we do `strip` afterwards, the `\s*` part in the pattern does not seem to be needed? (Wildcards incur a minor performance penalty.)
An alternative is to do `\s*` at both sides and remove the extra `strip`.
--
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] stamixthereal commented on pull request #28634: Add doc-strings and small improvement to email util
Posted by GitBox <gi...@apache.org>.
stamixthereal commented on PR #28634:
URL: https://github.com/apache/airflow/pull/28634#issuecomment-1367133320
Thanks for your review, @uranusjr; I fixed 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