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/03/17 19:49:08 UTC
[GitHub] [airflow] vincbeck opened a new pull request #22351: ImapAttachmentToS3Operator: fix it, update sample dag and update doc
vincbeck opened a new pull request #22351:
URL: https://github.com/apache/airflow/pull/22351
Fix the `ImapAttachmentToS3Operator`. I also updated the sample dag and doc associated
--
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] vincbeck commented on a change in pull request #22351: ImapAttachmentToS3Operator: fix it, update sample dag and update doc
Posted by GitBox <gi...@apache.org>.
vincbeck commented on a change in pull request #22351:
URL: https://github.com/apache/airflow/pull/22351#discussion_r829473426
##########
File path: airflow/providers/amazon/aws/transfers/imap_attachment_to_s3.py
##########
@@ -51,24 +52,26 @@ def __init__(
self,
*,
imap_attachment_name: str,
+ s3_bucket: str,
s3_key: str,
imap_check_regex: bool = False,
imap_mail_folder: str = 'INBOX',
imap_mail_filter: str = 'All',
s3_overwrite: bool = False,
imap_conn_id: str = 'imap_default',
- s3_conn_id: str = 'aws_default',
+ aws_conn_id: str = 'aws_default',
**kwargs,
) -> None:
super().__init__(**kwargs)
self.imap_attachment_name = imap_attachment_name
+ self.s3_bucket = s3_bucket
self.s3_key = s3_key
self.imap_check_regex = imap_check_regex
self.imap_mail_folder = imap_mail_folder
self.imap_mail_filter = imap_mail_filter
self.s3_overwrite = s3_overwrite
self.imap_conn_id = imap_conn_id
- self.s3_conn_id = s3_conn_id
+ self.aws_conn_id = aws_conn_id
Review comment:
Very good point! I totally miss that. Let me roll back that change. I just changed it for cosmetic details, I dont think it's necessary to raise a deprecation warning
--
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] vincbeck commented on a change in pull request #22351: ImapAttachmentToS3Operator: fix it, update sample dag and update doc
Posted by GitBox <gi...@apache.org>.
vincbeck commented on a change in pull request #22351:
URL: https://github.com/apache/airflow/pull/22351#discussion_r830325120
##########
File path: airflow/providers/amazon/aws/transfers/imap_attachment_to_s3.py
##########
@@ -51,24 +52,26 @@ def __init__(
self,
*,
imap_attachment_name: str,
+ s3_bucket: str,
s3_key: str,
imap_check_regex: bool = False,
imap_mail_folder: str = 'INBOX',
imap_mail_filter: str = 'All',
s3_overwrite: bool = False,
imap_conn_id: str = 'imap_default',
- s3_conn_id: str = 'aws_default',
+ aws_conn_id: str = 'aws_default',
**kwargs,
) -> None:
super().__init__(**kwargs)
self.imap_attachment_name = imap_attachment_name
+ self.s3_bucket = s3_bucket
self.s3_key = s3_key
self.imap_check_regex = imap_check_regex
self.imap_mail_folder = imap_mail_folder
self.imap_mail_filter = imap_mail_filter
self.s3_overwrite = s3_overwrite
self.imap_conn_id = imap_conn_id
- self.s3_conn_id = s3_conn_id
+ self.aws_conn_id = aws_conn_id
Review comment:
Done
--
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] eladkal commented on a change in pull request #22351: ImapAttachmentToS3Operator: fix it, update sample dag and update doc
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #22351:
URL: https://github.com/apache/airflow/pull/22351#discussion_r829449487
##########
File path: airflow/providers/amazon/aws/transfers/imap_attachment_to_s3.py
##########
@@ -51,24 +52,26 @@ def __init__(
self,
*,
imap_attachment_name: str,
+ s3_bucket: str,
s3_key: str,
imap_check_regex: bool = False,
imap_mail_folder: str = 'INBOX',
imap_mail_filter: str = 'All',
s3_overwrite: bool = False,
imap_conn_id: str = 'imap_default',
- s3_conn_id: str = 'aws_default',
+ aws_conn_id: str = 'aws_default',
**kwargs,
) -> None:
super().__init__(**kwargs)
self.imap_attachment_name = imap_attachment_name
+ self.s3_bucket = s3_bucket
self.s3_key = s3_key
self.imap_check_regex = imap_check_regex
self.imap_mail_folder = imap_mail_folder
self.imap_mail_filter = imap_mail_filter
self.s3_overwrite = s3_overwrite
self.imap_conn_id = imap_conn_id
- self.s3_conn_id = s3_conn_id
+ self.aws_conn_id = aws_conn_id
Review comment:
This is a breaking change. We can't just remove it.
We should raise deprecation warning. See for example:
https://github.com/apache/airflow/blob/602abe8394fafe7de54df7e73af56de848cdf617/airflow/providers/amazon/aws/transfers/mongo_to_s3.py#L78-L80
--
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] vincbeck commented on a change in pull request #22351: ImapAttachmentToS3Operator: fix it, update sample dag and update doc
Posted by GitBox <gi...@apache.org>.
vincbeck commented on a change in pull request #22351:
URL: https://github.com/apache/airflow/pull/22351#discussion_r831236538
##########
File path: airflow/providers/amazon/aws/transfers/imap_attachment_to_s3.py
##########
@@ -51,24 +57,31 @@ def __init__(
self,
*,
imap_attachment_name: str,
+ s3_bucket: str,
s3_key: str,
imap_check_regex: bool = False,
imap_mail_folder: str = 'INBOX',
imap_mail_filter: str = 'All',
s3_overwrite: bool = False,
imap_conn_id: str = 'imap_default',
- s3_conn_id: str = 'aws_default',
+ s3_conn_id: Optional[str] = None,
Review comment:
To raise the warning in case it is used. Check line 72
--
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] eladkal commented on a change in pull request #22351: ImapAttachmentToS3Operator: fix it, update sample dag and update doc
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #22351:
URL: https://github.com/apache/airflow/pull/22351#discussion_r829449487
##########
File path: airflow/providers/amazon/aws/transfers/imap_attachment_to_s3.py
##########
@@ -51,24 +52,26 @@ def __init__(
self,
*,
imap_attachment_name: str,
+ s3_bucket: str,
s3_key: str,
imap_check_regex: bool = False,
imap_mail_folder: str = 'INBOX',
imap_mail_filter: str = 'All',
s3_overwrite: bool = False,
imap_conn_id: str = 'imap_default',
- s3_conn_id: str = 'aws_default',
+ aws_conn_id: str = 'aws_default',
**kwargs,
) -> None:
super().__init__(**kwargs)
self.imap_attachment_name = imap_attachment_name
+ self.s3_bucket = s3_bucket
self.s3_key = s3_key
self.imap_check_regex = imap_check_regex
self.imap_mail_folder = imap_mail_folder
self.imap_mail_filter = imap_mail_filter
self.s3_overwrite = s3_overwrite
self.imap_conn_id = imap_conn_id
- self.s3_conn_id = s3_conn_id
+ self.aws_conn_id = aws_conn_id
Review comment:
This is a breaking change. We can't just remove it.
We should raise deprecation warning. See for example:
https://github.com/apache/airflow/blob/602abe8394fafe7de54df7e73af56de848cdf617/airflow/providers/amazon/aws/transfers/mongo_to_s3.py#L78-L80
##########
File path: airflow/providers/amazon/aws/transfers/imap_attachment_to_s3.py
##########
@@ -51,24 +52,26 @@ def __init__(
self,
*,
imap_attachment_name: str,
+ s3_bucket: str,
s3_key: str,
imap_check_regex: bool = False,
imap_mail_folder: str = 'INBOX',
imap_mail_filter: str = 'All',
s3_overwrite: bool = False,
imap_conn_id: str = 'imap_default',
- s3_conn_id: str = 'aws_default',
+ aws_conn_id: str = 'aws_default',
**kwargs,
) -> None:
super().__init__(**kwargs)
self.imap_attachment_name = imap_attachment_name
+ self.s3_bucket = s3_bucket
self.s3_key = s3_key
self.imap_check_regex = imap_check_regex
self.imap_mail_folder = imap_mail_folder
self.imap_mail_filter = imap_mail_filter
self.s3_overwrite = s3_overwrite
self.imap_conn_id = imap_conn_id
- self.s3_conn_id = s3_conn_id
+ self.aws_conn_id = aws_conn_id
Review comment:
This is a breaking change. We can't just rename it.
We should raise deprecation warning. See for example:
https://github.com/apache/airflow/blob/602abe8394fafe7de54df7e73af56de848cdf617/airflow/providers/amazon/aws/transfers/mongo_to_s3.py#L78-L80
##########
File path: airflow/providers/amazon/aws/transfers/imap_attachment_to_s3.py
##########
@@ -51,24 +52,26 @@ def __init__(
self,
*,
imap_attachment_name: str,
+ s3_bucket: str,
s3_key: str,
imap_check_regex: bool = False,
imap_mail_folder: str = 'INBOX',
imap_mail_filter: str = 'All',
s3_overwrite: bool = False,
imap_conn_id: str = 'imap_default',
- s3_conn_id: str = 'aws_default',
+ aws_conn_id: str = 'aws_default',
**kwargs,
) -> None:
super().__init__(**kwargs)
self.imap_attachment_name = imap_attachment_name
+ self.s3_bucket = s3_bucket
self.s3_key = s3_key
self.imap_check_regex = imap_check_regex
self.imap_mail_folder = imap_mail_folder
self.imap_mail_filter = imap_mail_filter
self.s3_overwrite = s3_overwrite
self.imap_conn_id = imap_conn_id
- self.s3_conn_id = s3_conn_id
+ self.aws_conn_id = aws_conn_id
Review comment:
The change it good and needed :)
Just do with with warning
--
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 #22351: ImapAttachmentToS3Operator: fix it, update sample dag and update doc
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #22351:
URL: https://github.com/apache/airflow/pull/22351#issuecomment-1074439310
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.
--
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] vincbeck commented on a change in pull request #22351: ImapAttachmentToS3Operator: fix it, update sample dag and update doc
Posted by GitBox <gi...@apache.org>.
vincbeck commented on a change in pull request #22351:
URL: https://github.com/apache/airflow/pull/22351#discussion_r829473426
##########
File path: airflow/providers/amazon/aws/transfers/imap_attachment_to_s3.py
##########
@@ -51,24 +52,26 @@ def __init__(
self,
*,
imap_attachment_name: str,
+ s3_bucket: str,
s3_key: str,
imap_check_regex: bool = False,
imap_mail_folder: str = 'INBOX',
imap_mail_filter: str = 'All',
s3_overwrite: bool = False,
imap_conn_id: str = 'imap_default',
- s3_conn_id: str = 'aws_default',
+ aws_conn_id: str = 'aws_default',
**kwargs,
) -> None:
super().__init__(**kwargs)
self.imap_attachment_name = imap_attachment_name
+ self.s3_bucket = s3_bucket
self.s3_key = s3_key
self.imap_check_regex = imap_check_regex
self.imap_mail_folder = imap_mail_folder
self.imap_mail_filter = imap_mail_filter
self.s3_overwrite = s3_overwrite
self.imap_conn_id = imap_conn_id
- self.s3_conn_id = s3_conn_id
+ self.aws_conn_id = aws_conn_id
Review comment:
Very good point! I totally miss that. Let me roll back that change. I just changed it for cosmetic details, I dont think it's necessary to raise a deprecation warning
--
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] eladkal commented on a change in pull request #22351: ImapAttachmentToS3Operator: fix it, update sample dag and update doc
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #22351:
URL: https://github.com/apache/airflow/pull/22351#discussion_r829481679
##########
File path: airflow/providers/amazon/aws/transfers/imap_attachment_to_s3.py
##########
@@ -51,24 +52,26 @@ def __init__(
self,
*,
imap_attachment_name: str,
+ s3_bucket: str,
s3_key: str,
imap_check_regex: bool = False,
imap_mail_folder: str = 'INBOX',
imap_mail_filter: str = 'All',
s3_overwrite: bool = False,
imap_conn_id: str = 'imap_default',
- s3_conn_id: str = 'aws_default',
+ aws_conn_id: str = 'aws_default',
**kwargs,
) -> None:
super().__init__(**kwargs)
self.imap_attachment_name = imap_attachment_name
+ self.s3_bucket = s3_bucket
self.s3_key = s3_key
self.imap_check_regex = imap_check_regex
self.imap_mail_folder = imap_mail_folder
self.imap_mail_filter = imap_mail_filter
self.s3_overwrite = s3_overwrite
self.imap_conn_id = imap_conn_id
- self.s3_conn_id = s3_conn_id
+ self.aws_conn_id = aws_conn_id
Review comment:
The change it good and needed :)
Just do with with warning
--
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] eladkal commented on a change in pull request #22351: ImapAttachmentToS3Operator: fix it, update sample dag and update doc
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #22351:
URL: https://github.com/apache/airflow/pull/22351#discussion_r829449487
##########
File path: airflow/providers/amazon/aws/transfers/imap_attachment_to_s3.py
##########
@@ -51,24 +52,26 @@ def __init__(
self,
*,
imap_attachment_name: str,
+ s3_bucket: str,
s3_key: str,
imap_check_regex: bool = False,
imap_mail_folder: str = 'INBOX',
imap_mail_filter: str = 'All',
s3_overwrite: bool = False,
imap_conn_id: str = 'imap_default',
- s3_conn_id: str = 'aws_default',
+ aws_conn_id: str = 'aws_default',
**kwargs,
) -> None:
super().__init__(**kwargs)
self.imap_attachment_name = imap_attachment_name
+ self.s3_bucket = s3_bucket
self.s3_key = s3_key
self.imap_check_regex = imap_check_regex
self.imap_mail_folder = imap_mail_folder
self.imap_mail_filter = imap_mail_filter
self.s3_overwrite = s3_overwrite
self.imap_conn_id = imap_conn_id
- self.s3_conn_id = s3_conn_id
+ self.aws_conn_id = aws_conn_id
Review comment:
This is a breaking change. We can't just rename it.
We should raise deprecation warning. See for example:
https://github.com/apache/airflow/blob/602abe8394fafe7de54df7e73af56de848cdf617/airflow/providers/amazon/aws/transfers/mongo_to_s3.py#L78-L80
--
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] eladkal commented on a change in pull request #22351: ImapAttachmentToS3Operator: fix it, update sample dag and update doc
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #22351:
URL: https://github.com/apache/airflow/pull/22351#discussion_r831563773
##########
File path: docs/apache-airflow-providers-amazon/operators/transfer/imap_attachment_to_s3.rst
##########
@@ -0,0 +1,44 @@
+ .. Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ .. http://www.apache.org/licenses/LICENSE-2.0
+
+ .. Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+
+Imap Attachment To Amazon S3 Operator
+=====================================
+
+The ``ImapAttachmentToS3Operator`` transfers an email attachment via IMAP
+protocol from a mail server to an Amazon S3 Bucket.
+
+Prerequisite Tasks
+^^^^^^^^^^^^^^^^^^
+
+.. include:: ../_partials/prerequisite_tasks.rst
+
+.. _howto/operator:ImapAttachmentToS3Operator:
+
+Imap Attachment To Amazon S3
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+.. exampleinclude:: /../../airflow/providers/amazon/aws/example_dags/example_imap_attachment_to_s3.py
+ :language: python
+ :dedent: 4
+ :start-after: [START howto_transfer_imap_attachment_to_s3]
+ :end-before: [END howto_transfer_imap_attachment_to_s3]
+
+Reference
+^^^^^^^^^
+
+* `IMAP Library Documentation <https://docs.python.org/3.6/library/imaplib.html>`__
Review comment:
```suggestion
* `IMAP Library Documentation <https://docs.python.org/3/library/imaplib.html>`__
```
lets refer to latest python rather than to 3.6
--
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] eladkal commented on a change in pull request #22351: ImapAttachmentToS3Operator: fix it, update sample dag and update doc
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #22351:
URL: https://github.com/apache/airflow/pull/22351#discussion_r830388744
##########
File path: airflow/providers/amazon/aws/transfers/imap_attachment_to_s3.py
##########
@@ -51,24 +57,31 @@ def __init__(
self,
*,
imap_attachment_name: str,
+ s3_bucket: str,
s3_key: str,
imap_check_regex: bool = False,
imap_mail_folder: str = 'INBOX',
imap_mail_filter: str = 'All',
s3_overwrite: bool = False,
imap_conn_id: str = 'imap_default',
- s3_conn_id: str = 'aws_default',
+ s3_conn_id: Optional[str] = None,
Review comment:
What is the reason for changing the default?
--
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] eladkal commented on a change in pull request #22351: ImapAttachmentToS3Operator: fix it, update sample dag and update doc
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #22351:
URL: https://github.com/apache/airflow/pull/22351#discussion_r830388744
##########
File path: airflow/providers/amazon/aws/transfers/imap_attachment_to_s3.py
##########
@@ -51,24 +57,31 @@ def __init__(
self,
*,
imap_attachment_name: str,
+ s3_bucket: str,
s3_key: str,
imap_check_regex: bool = False,
imap_mail_folder: str = 'INBOX',
imap_mail_filter: str = 'All',
s3_overwrite: bool = False,
imap_conn_id: str = 'imap_default',
- s3_conn_id: str = 'aws_default',
+ s3_conn_id: Optional[str] = None,
Review comment:
What is the reason to change the default?
--
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] vincbeck commented on a change in pull request #22351: ImapAttachmentToS3Operator: fix it, update sample dag and update doc
Posted by GitBox <gi...@apache.org>.
vincbeck commented on a change in pull request #22351:
URL: https://github.com/apache/airflow/pull/22351#discussion_r831236538
##########
File path: airflow/providers/amazon/aws/transfers/imap_attachment_to_s3.py
##########
@@ -51,24 +57,31 @@ def __init__(
self,
*,
imap_attachment_name: str,
+ s3_bucket: str,
s3_key: str,
imap_check_regex: bool = False,
imap_mail_folder: str = 'INBOX',
imap_mail_filter: str = 'All',
s3_overwrite: bool = False,
imap_conn_id: str = 'imap_default',
- s3_conn_id: str = 'aws_default',
+ s3_conn_id: Optional[str] = None,
Review comment:
To raise a warning in case it is used. Check line 72
--
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 #22351: ImapAttachmentToS3Operator: fix it, update sample dag and update doc
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #22351:
URL: https://github.com/apache/airflow/pull/22351
--
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