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/12/09 12:28:29 UTC
[GitHub] [airflow] KennyRich opened a new pull request #20167: Organise S3 Classes in Amazon Provider
KennyRich opened a new pull request #20167:
URL: https://github.com/apache/airflow/pull/20167
Part of #20139
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/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/main/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.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on pull request #20167: Organise S3 Classes in Amazon Provider
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #20167:
URL: https://github.com/apache/airflow/pull/20167#issuecomment-992587597
Awesome work, congrats on your first merged pull request!
--
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] boring-cyborg[bot] commented on pull request #20167: Organise S3 Classes in Amazon Provider
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #20167:
URL: https://github.com/apache/airflow/pull/20167#issuecomment-989807041
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (flake8, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/BREEZE.rst) for testing locally, itโs a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#coding-style-and-best-practices).
Apache Airflow is a community-driven project and together we are making it better ๐.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
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 pull request #20167: Organise S3 Classes in Amazon Provider
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #20167:
URL: https://github.com/apache/airflow/pull/20167#issuecomment-990054748
@KennyRich can you please rebase & resolve conflicts?
Also note that some of the S3 operators are also in contrib.
For example:
https://github.com/apache/airflow/blob/f4e04c713f56b6cd95d9e88c53f577f785838a83/tests/deprecated_classes.py#L1067-L1068
This means that contrib (which is deprecated) is now pointing to deprecated class. This is probably wrong and should direct to the new class directly(?)
--
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] KennyRich commented on pull request #20167: Organise S3 Classes in Amazon Provider
Posted by GitBox <gi...@apache.org>.
KennyRich commented on pull request #20167:
URL: https://github.com/apache/airflow/pull/20167#issuecomment-992440712
> LGTM Will merge once CI is green.
>
> @KennyRich can you please rebase and resolve conflicts?
I've resolved the merge conflicts. Thanks for your help so far!
--
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 #20167: Organise S3 Classes in Amazon Provider
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #20167:
URL: https://github.com/apache/airflow/pull/20167#discussion_r766580700
##########
File path: airflow/providers/amazon/aws/operators/s3_bucket_tagging.py
##########
@@ -15,139 +15,17 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
-"""This module contains AWS S3 operators."""
-from typing import Dict, List, Optional
-
-from airflow.models import BaseOperator
-from airflow.providers.amazon.aws.hooks.s3 import S3Hook
-
-BUCKET_DOES_NOT_EXIST_MSG = "Bucket with name: %s doesn't exist"
-
-
-class S3GetBucketTaggingOperator(BaseOperator):
- """
- This operator gets tagging from an S3 bucket
-
- .. seealso::
- For more information on how to use this operator, take a look at the guide:
- :ref:`howto/operator:S3GetBucketTaggingOperator`
-
- :param bucket_name: This is bucket name you want to reference
- :type bucket_name: str
- :param aws_conn_id: The Airflow connection used for AWS credentials.
- If this is None or empty then the default boto3 behaviour is used. If
- running Airflow in a distributed manner and aws_conn_id is None or
- empty, then default boto3 configuration would be used (and must be
- maintained on each worker node).
- :type aws_conn_id: Optional[str]
- """
-
- template_fields = ("bucket_name",)
-
- def __init__(self, bucket_name: str, aws_conn_id: Optional[str] = "aws_default", **kwargs) -> None:
- super().__init__(**kwargs)
- self.bucket_name = bucket_name
- self.aws_conn_id = aws_conn_id
-
- def execute(self, context):
- s3_hook = S3Hook(aws_conn_id=self.aws_conn_id)
-
- if s3_hook.check_for_bucket(self.bucket_name):
- self.log.info("Getting tags for bucket %s", self.bucket_name)
- return s3_hook.get_bucket_tagging(self.bucket_name)
- else:
- self.log.warning(BUCKET_DOES_NOT_EXIST_MSG, self.bucket_name)
- return None
-
-
-class S3PutBucketTaggingOperator(BaseOperator):
- """
- This operator puts tagging for an S3 bucket.
-
- .. seealso::
- For more information on how to use this operator, take a look at the guide:
- :ref:`howto/operator:S3PutBucketTaggingOperator`
-
- :param bucket_name: The name of the bucket to add tags to.
- :type bucket_name: str
- :param key: The key portion of the key/value pair for a tag to be added.
- If a key is provided, a value must be provided as well.
- :type key: str
- :param value: The value portion of the key/value pair for a tag to be added.
- If a value is provided, a key must be provided as well.
- :param tag_set: A List of key/value pairs.
- :type tag_set: List[Dict[str, str]]
- :param aws_conn_id: The Airflow connection used for AWS credentials.
- If this is None or empty then the default boto3 behaviour is used. If
- running Airflow in a distributed manner and aws_conn_id is None or
- empty, then the default boto3 configuration would be used (and must be
- maintained on each worker node).
- :type aws_conn_id: Optional[str]
- """
-
- template_fields = ("bucket_name",)
- template_fields_renderers = {"tag_set": "json"}
-
- def __init__(
- self,
- bucket_name: str,
- key: Optional[str] = None,
- value: Optional[str] = None,
- tag_set: Optional[List[Dict[str, str]]] = None,
- aws_conn_id: Optional[str] = "aws_default",
- **kwargs,
- ) -> None:
- super().__init__(**kwargs)
- self.key = key
- self.value = value
- self.tag_set = tag_set
- self.bucket_name = bucket_name
- self.aws_conn_id = aws_conn_id
-
- def execute(self, context):
- s3_hook = S3Hook(aws_conn_id=self.aws_conn_id)
-
- if s3_hook.check_for_bucket(self.bucket_name):
- self.log.info("Putting tags for bucket %s", self.bucket_name)
- return s3_hook.put_bucket_tagging(
- key=self.key, value=self.value, tag_set=self.tag_set, bucket_name=self.bucket_name
- )
- else:
- self.log.warning(BUCKET_DOES_NOT_EXIST_MSG, self.bucket_name)
- return None
-
-
-class S3DeleteBucketTaggingOperator(BaseOperator):
- """
- This operator deletes tagging from an S3 bucket.
-
- .. seealso::
- For more information on how to use this operator, take a look at the guide:
- :ref:`howto/operator:S3DeleteBucketTaggingOperator`
-
- :param bucket_name: This is the name of the bucket to delete tags from.
- :type bucket_name: str
- :param aws_conn_id: The Airflow connection used for AWS credentials.
- If this is None or empty then the default boto3 behaviour is used. If
- running Airflow in a distributed manner and aws_conn_id is None or
- empty, then default boto3 configuration would be used (and must be
- maintained on each worker node).
- :type aws_conn_id: Optional[str]
- """
-
- template_fields = ("bucket_name",)
-
- def __init__(self, bucket_name: str, aws_conn_id: Optional[str] = "aws_default", **kwargs) -> None:
- super().__init__(**kwargs)
- self.bucket_name = bucket_name
- self.aws_conn_id = aws_conn_id
-
- def execute(self, context):
- s3_hook = S3Hook(aws_conn_id=self.aws_conn_id)
-
- if s3_hook.check_for_bucket(self.bucket_name):
- self.log.info("Deleting tags for bucket %s", self.bucket_name)
- return s3_hook.delete_bucket_tagging(self.bucket_name)
- else:
- self.log.warning(BUCKET_DOES_NOT_EXIST_MSG, self.bucket_name)
- return None
+"""This module is deprecated. Please use :mod:`airflow.providers.amazon.aws.operators.s3`."""
+
+import warnings
+
+from airflow.providers.amazon.aws.operators.s3 import ( # noqa
+ S3DeleteBucketTaggingOperator,
+ S3GetBucketTaggingOperator,
+ S3PutBucketTaggingOperator,
+)
+warnings.warn(
Review comment:
```suggestion
S3PutBucketTaggingOperator,
)
warnings.warn(
```
missing empty line to make static checks happy
--
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 merged pull request #20167: Organise S3 Classes in Amazon Provider
Posted by GitBox <gi...@apache.org>.
eladkal merged pull request #20167:
URL: https://github.com/apache/airflow/pull/20167
--
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 #20167: Organise S3 Classes in Amazon Provider
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #20167:
URL: https://github.com/apache/airflow/pull/20167#discussion_r766045488
##########
File path: tests/deprecated_classes.py
##########
@@ -1056,17 +1056,21 @@
'airflow.contrib.operators.emr_terminate_job_flow_operator.EmrTerminateJobFlowOperator',
),
(
- 'airflow.providers.amazon.aws.operators.s3_copy_object.S3CopyObjectOperator',
+ 'airflow.providers.amazon.aws.operators.S3CopyObjectOperator',
'airflow.contrib.operators.s3_copy_object_operator.S3CopyObjectOperator',
),
(
- 'airflow.providers.amazon.aws.operators.s3_delete_objects.S3DeleteObjectsOperator',
+ 'airflow.providers.amazon.aws.operators.S3DeleteObjectsOperator',
Review comment:
```suggestion
'airflow.providers.amazon.aws.operators.s3.S3DeleteObjectsOperator',
```
##########
File path: tests/deprecated_classes.py
##########
@@ -1056,17 +1056,21 @@
'airflow.contrib.operators.emr_terminate_job_flow_operator.EmrTerminateJobFlowOperator',
),
(
- 'airflow.providers.amazon.aws.operators.s3_copy_object.S3CopyObjectOperator',
+ 'airflow.providers.amazon.aws.operators.S3CopyObjectOperator',
Review comment:
```suggestion
'airflow.providers.amazon.aws.operators.s3.S3CopyObjectOperator',
```
##########
File path: tests/deprecated_classes.py
##########
@@ -1056,17 +1056,21 @@
'airflow.contrib.operators.emr_terminate_job_flow_operator.EmrTerminateJobFlowOperator',
),
(
- 'airflow.providers.amazon.aws.operators.s3_copy_object.S3CopyObjectOperator',
+ 'airflow.providers.amazon.aws.operators.S3CopyObjectOperator',
'airflow.contrib.operators.s3_copy_object_operator.S3CopyObjectOperator',
),
(
- 'airflow.providers.amazon.aws.operators.s3_delete_objects.S3DeleteObjectsOperator',
+ 'airflow.providers.amazon.aws.operators.S3DeleteObjectsOperator',
'airflow.contrib.operators.s3_delete_objects_operator.S3DeleteObjectsOperator',
),
(
- 'airflow.providers.amazon.aws.operators.s3_list.S3ListOperator',
+ 'airflow.providers.amazon.aws.operators.S3ListOperator',
Review comment:
```suggestion
'airflow.providers.amazon.aws.operators.s3.S3ListOperator',
```
--
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 pull request #20167: Organise S3 Classes in Amazon Provider
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #20167:
URL: https://github.com/apache/airflow/pull/20167#issuecomment-991863258
@KennyRich you are missing also fixes to test classes:
https://github.com/apache/airflow/tree/main/tests/providers/amazon/aws/operators
https://github.com/apache/airflow/tree/main/tests/providers/amazon/aws/sensors
Tests are still importing operators from old paths.
--
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] KennyRich commented on pull request #20167: Organise S3 Classes in Amazon Provider
Posted by GitBox <gi...@apache.org>.
KennyRich commented on pull request #20167:
URL: https://github.com/apache/airflow/pull/20167#issuecomment-990063687
> @KennyRich can you please rebase & resolve conflicts? Also note that some of the S3 operators are also in contrib. For example:
>
> https://github.com/apache/airflow/blob/f4e04c713f56b6cd95d9e88c53f577f785838a83/tests/deprecated_classes.py#L1067-L1068
>
> This means that contrib (which is deprecated) is now pointing to deprecated class. This is probably wrong and should direct to the new class directly(?)
@eladkal Thanks for the feedback, I'll do that and send in a PR
--
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