Repository navigation
Consistently name Amazon provider operators files #20296
Description
Description
Follow-up to
Restructure Amazon provider operators files #20139 as discussed with @eladkal
Amazon Operators, Hooks, and Sensors are inconsistently named regarding acronym capitalization and whether to prefix with AWS or not.
An incomplete list for example: we have SnsPublishOperator but we capitalize the acronyms in SQSPublishOperator and ECSOperator. We prefix AwsGlueCrawlerOperator and AwsBatchOperator in lowercase, AWSAthenaOperator is prefixed in uppercase, and no prefix at all on GlacierCreateJobOperatoror CloudFormationCreateStackOperator
Correct naming conventions are listed in AIP 21 cases 3-4 and case 6.
The sensors/operators class names should not have Aws/AWS as part of the class name and only the first letter of the acronyms should be capitalized.
So:
SnsPublishOperator -> No change
SQSPublishOperator -> SqsPublishOperator
AwsBatchOperator -> BatchOperator
AWSAthenaOperator -> AthenaOperator
etc...
Use case/motivation
For each task:
-Rename the operator
-Deprecate the old name
--Add to tests/deprecated_classes.py
--Add to the KNOWN_DEPRECATED_MESSAGES section of dev/provider_packages/prepare_provider_packages.py
-Correct any references to the old name in docstrings and docs files
-Correct all references in tests
-Correct references in example dags
Changes Required:
| Current | New Name |
|---|---|
| AWSAthenaOperator | AthenaOperator |
| AwsBatchOperator | BatchOperator |
| AWSDataSyncOperator | DataSyncOperator |
| ECSProtocol | EcsProtocol |
| ECSTaskLogFetcher | EcsTaskLogFetcher |
| ECSOperator | EcsOperator |
| EKS***Operator (all of them) | Eks***Operator |
| SQSPublishOperator | SqsPublishOperator |
| EKS***Sensor (all of them) | Eks***Sensor |
| AwsRedshiftClusterSensor | RedshiftClusterSensor |
| SQSSensor | SqsSensor |
| AWSAthenaHook | AthenaHook |
| AWSCloudFormationHook | CloudFormationHook |
| AWSDataSyncHook | DataSyncHook |
| AwsDynamoDBHook | DynamoDBHook |
| EKSHook | EksHook |
| AwsFirehoseHook (in kinesis.py) | FirehoseHook |
| AwsLambdaHook | LambdaHook |
| SESHook | SesHook |
| AwsSnsHook | SnsHook |
| SQSHook | SqsHook |
Action Items:
- operators/athena.py - @ferruzzi Standardize AWS Athena naming #20305
- operators/batch.py - @ferruzzi Standardize AWS Batch naming #20369
- operators/datasync.py - @ferruzzi Standardize AWS DataSync naming #20328
- operators/ecs.py - @ferruzzi Standardize AWS ECS naming #20332
- operators/eks.py - @ferruzzi Standardize AWS EKS naming #20354
- operators/glue_crawler.py - @ferruzzi Standardize AWS Glue naming #20372
- operators/sqs.py - @eladkal Standardize AWS SQS classes names #20732
- sensors/eks.py - @ferruzzi Standardize AWS EKS naming #20354
- sensors/redshift.py - @ferruzzi Standardize AWS Redshift naming #20374
- sensors/sqs.py - @eladkal Standardize AWS SQS classes names #20732
- hooks/athena.py - @ferruzzi Standardize AWS Athena naming #20305
- hooks/cloud_formation.py @ferruzzi Standardize AWS CloudFormation naming #20357
- hooks/datasync.py - @ferruzzi Standardize AWS DataSync naming #20328
- hooks/dynamodb.py - @ferruzzi Standardize DynamoDB naming #20360
- hooks/emr.py - @bhavaniravi 20139 - move emr_container hook #20375
- hooks/eks.py - @ferruzzi Standardize AWS EKS naming #20354
- hooks/kinesis.py - @ferruzzi Standardize AWS Kinesis/Firehose naming #20362
- hooks/lambda.py - @ferruzzi Standardize AWS Lambda naming #20365
- hooks/ses.py - @ferruzzi Standardize Amazon SES naming #20367
- hooks/sns.py - @ferruzzi Standardize Amazon SNS naming #20368
- hooks/sqs.py - @eladkal Standardize AWS SQS classes names #20732
To be verified (recently changed in the linked campaign):
DMS - good
EC2 - good
EMR - good
Glue - Added above
S3 - good
SageMaker - good
No changes required:
Operators:
CloudFormationCreateStackOperator
CloudFormationDeleteStackOperator
GlacierCreateJobOperator
SnsPublishOperator
StepFunctionGetExecutionOutputOperator
StepFunctionStartExecutionOperator
AthenaSensor
CloudFormationCreateStackSensor
CloudFormationDeleteStackSensor
DmsTaskBaseSensor
DmsTaskCompletedSensor
Sensors:
GlacierJobOperationSensor
StepFunctionExecutionSensor
Hooks:
ElastiCacheReplicationGroupHook
GlacierHook
RedshiftHook
S3Hook
SageMakerHook
SecretsManagerHook
StepFunctionHook
Related issues
Are you willing to submit a PR?
- Yes I am willing to submit a PR!
Code of Conduct
- I agree to follow this project's Code of Conduct
Activity
ferruzzi commentedon Dec 14, 2021
@eladkal Should the Hooks be part of this campaign as well, or is that scope-creep getting out of hand?
eladkal commentedon Dec 14, 2021
We should do both
#20160 already handle Emr
This task shouldn't take long. Lets try to finish all in the next few days so all new paths/classes will be avaliable in the next peovider release.
eladkal commentedon Dec 14, 2021
@KennyRich would you like to pick some?
ferruzzi commentedon Dec 14, 2021
Sounds good. I'll add the hooks to the Action Items.
I rebased 'main' this morning and it was failing pre-commits. I just re-rebased and running them again, but I think I have Athena operators done, I'll get a PR for those up once local CI passes.
[EDIT =
mainis still failing pre-commits as of 12:45 PST]ferruzzi commentedon Dec 14, 2021
hooks/logs.py calls the hook AwsLogsHook and... I think we should keep that one. "LogsHook" is just too generic IMHO.
KennyRich commentedon Dec 14, 2021
Yes sure!!!
dstandish commentedon Dec 14, 2021
@eladkal i thought that
RedshiftSqlOperatorwas not correct and thatRedshiftSQLOperator.... i remember discussion about this at some point in the pasteladkal commentedon Dec 14, 2021
I think so too. I didnt review the whole list yet.
We always must look for consistency (other operators, other providers etc...) In case of sql its easy as we have planty.
ferruzzi commentedon Dec 14, 2021
My mistake! I have updated the OP
eladkal commentedon Dec 15, 2021
Please let us know which so we can assign.
KennyRich commentedon Dec 15, 2021
I'll handle Sqs
eladkal commentedon Dec 16, 2021
@bhavaniravi would you like to take the emr hook?
bhavaniravi commentedon Dec 16, 2021
Yup will do.
ferruzzi commentedon Dec 16, 2021
EKS is a big one, I'll get started on that
ferruzzi commentedon Dec 17, 2021
Question for @eladkal - The module for the Lambda hook is named
hooks/lambda_function.py. The class is AwsLambdaHook (being renamed to LambdaHook). Do we want to rename the module to justlambda.pywhile we're at it? Or maybe make the hook LambdaFunctionHook so it matches the module name? In my experience, "nobody" calls it an AWS Lambda Function in conversation, I always hear it referred to as AWS Lambda, so I don't think that would be a confusing change? Counterpoint: the Dynamo module is still calleddynamodb.pyand I don't feel like that one should be changed, so I'm not sure. Maybe just leave the file name alone? Thoughts?Rename AwsRedshiftClusterSensor to RedshiftClusterSensor
eladkal commentedon Dec 20, 2021
@KennyRich do you need help?
KennyRich commentedon Dec 20, 2021
Please you can help go through my PR