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/20 07:34:16 UTC
[GitHub] [airflow] eladkal commented on a change in pull request #20418: Standardize naming of AWS Sqs
eladkal commented on a change in pull request #20418:
URL: https://github.com/apache/airflow/pull/20418#discussion_r772133467
##########
File path: airflow/providers/amazon/aws/hooks/sqs.py
##########
@@ -22,7 +22,7 @@
from airflow.providers.amazon.aws.hooks.base_aws import AwsBaseHook
-class SQSHook(AwsBaseHook):
+class SqsHook(AwsBaseHook):
Review comment:
This is also not backward compatible. We want both `SQSHook` and `SqsHook` to work.
##########
File path: UPDATING.md
##########
@@ -1994,17 +1994,17 @@ Migrated are:
| airflow.hooks.S3_hook.S3Hook | airflow.providers.amazon.aws.hooks.s3.S3Hook |
| airflow.contrib.hooks.aws_athena_hook.AWSAthenaHook | airflow.providers.amazon.aws.hooks.athena.AWSAthenaHook |
| airflow.contrib.hooks.aws_lambda_hook.AwsLambdaHook | airflow.providers.amazon.aws.hooks.lambda_function.AwsLambdaHook |
-| airflow.contrib.hooks.aws_sqs_hook.SQSHook | airflow.providers.amazon.aws.hooks.sqs.SQSHook |
+| airflow.contrib.hooks.aws_sqs_hook.SqsHook | airflow.providers.amazon.aws.hooks.sqs.SqsHook |
Review comment:
There is no `SqsHook` in contrib.
We also can't suggest here `airflow.contrib.hooks.aws_sqs_hook.SQSHook` -> `airflow.providers.amazon.aws.hooks.sqs.SqsHook` because this table is for upgrading from 1.10 to 2.0 and in 2.0 only backport amazon provider is compatible so the changes we do in this PR are not relevant for it.
Long story short - please undo the changes you made for this file :)
##########
File path: CHANGELOG.txt
##########
@@ -2591,7 +2591,7 @@ Doc-only changes
- [AIRFLOW-XXX] update SlackWebhookHook and SlackWebhookOperator docstring (#5074)
- [AIRFLOW-XXX] Ignore Python files under node_modules in docs (#5063)
- [AIRFLOW-XXX] Build a universal wheel with LICNESE files (#5052)
-- [AIRFLOW-XXX] Fix docstrings of SQSHook (#5099)
+- [AIRFLOW-XXX] Fix docstrings of SqsHook (#5099)
Review comment:
We don't alter past records in change log. this entry was valid to the day it was created.
Please revert this change.
##########
File path: airflow/providers/amazon/CHANGELOG.rst
##########
@@ -253,7 +253,7 @@ Bug fixes
* ``Fix 'logging.exception' redundancy (#14823)``
* ``Fix AthenaSensor calling AthenaHook incorrectly (#15427)``
* ``Add links to new modules for deprecated modules (#15316)``
-* ``Fixes doc for SQSSensor (#15323)``
+* ``Fixes doc for SqsSensor (#15323)``
Review comment:
same comment as previous changelog.
Please undo
##########
File path: airflow/contrib/operators/aws_sqs_publish_operator.py
##########
@@ -19,7 +19,7 @@
import warnings
-from airflow.providers.amazon.aws.operators.sqs import SQSPublishOperator # noqa
+from airflow.providers.amazon.aws.operators.sqs import SqsPublishOperator # noqa
Review comment:
same
##########
File path: airflow/contrib/sensors/aws_sqs_sensor.py
##########
@@ -20,7 +20,7 @@
import warnings
-from airflow.providers.amazon.aws.sensors.sqs import SQSSensor # noqa
+from airflow.providers.amazon.aws.sensors.sqs import SqsSensor # noqa
Review comment:
same
##########
File path: airflow/contrib/hooks/aws_sqs_hook.py
##########
@@ -20,7 +20,7 @@
import warnings
-from airflow.providers.amazon.aws.hooks.sqs import SQSHook # noqa
+from airflow.providers.amazon.aws.hooks.sqs import SqsHook # noqa
Review comment:
This is not backward compatible. We are changing the class name.
So with the current code when user try to import SQSHook it won't work
Check reference of SESHook https://github.com/apache/airflow/pull/20367/files
It should
--
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