[AIRFLOW-6258] add CloudFormation operators to AWS providers #6824

aviemzur · 2019-12-15T12:28:37Z

Make sure you have checked all steps below.

Jira

My PR addresses the following Airflow Jira issues and references them in the PR title. For example, "[AIRFLOW-6258] My Airflow PR"
- https://issues.apache.org/jira/browse/AIRFLOW-6258
- In case you are fixing a typo in the documentation you can prepend your commit with [AIRFLOW-XXX], code changes always need a Jira issue.
- In case you are proposing a fundamental code change, you need to create an Airflow Improvement Proposal (AIP).
- In case you are adding a dependency, check if the license complies with the ASF 3rd Party License Policy.

Description

Here are some details about my PR, including screenshots of any UI changes:

Add 2 new operators: CloudFormationCreateStackOperator and CloudFormationDeleteStackOperator
https://issues.apache.org/jira/browse/AIRFLOW-6258

Tests

My PR adds the following unit tests OR does not need testing for this extremely good reason:

TestCloudFormationCreateStackOperator and TestCloudFormationDeleteStackOperator

Commits

My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
1. Subject is separated from body by a blank line
2. Subject is limited to 50 characters (not including Jira issue reference)
3. Subject does not end with a period
4. Subject uses the imperative mood ("add", not "adding")
5. Body wraps at 72 characters
6. Body explains "what" and "why", not "how"

Documentation

In case of new functionality, my PR adds documentation that describes how to use it.
- All the public functions and the classes in the PR contain docstrings that explain what it does
- If you implement backwards incompatible changes, please leave a note in the Updating.md so we can assign it to a appropriate release

OmerJog · 2019-12-17T08:02:33Z

@aviemzur can you please fix errors?

airflow/contrib/operators/cloudformation_create_stack_operator.py:1:0: C0111: Missing module docstring (missing-docstring)
airflow/contrib/operators/cloudformation_create_stack_operator.py:52:8: C0103: Attribute name "StackName" doesn't conform to snake_case naming style (invalid-name)
airflow/contrib/operators/cloudformation_create_stack_operator.py:53:8: C0103: Attribute name "TimeoutInMinutes" doesn't conform to snake_case naming style (invalid-name)
airflow/contrib/operators/cloudformation_create_stack_operator.py:49:57: E1101: Method '__init__' has no '__wrapped__' member (no-member)
airflow/contrib/operators/cloudformation_create_stack_operator.py:49:57: E1101: Method '__init__' has no '__wrapped__' member (no-member)
airflow/contrib/operators/cloudformation_create_stack_operator.py:49:57: E1101: Method '__init__' has no '__wrapped__' member (no-member)
************* Module airflow.contrib.operators.cloudformation_delete_stack_operator
airflow/contrib/operators/cloudformation_delete_stack_operator.py:1:0: C0111: Missing module docstring (missing-docstring)
airflow/contrib/operators/cloudformation_delete_stack_operator.py:48:8: C0103: Attribute name "StackName" doesn't conform to snake_case naming style (invalid-name)
airflow/contrib/operators/cloudformation_delete_stack_operator.py:45:57: E1101: Method '__init__' has no '__wrapped__' member (no-member)
airflow/contrib/operators/cloudformation_delete_stack_operator.py:45:57: E1101: Method '__init__' has no '__wrapped__' member (no-member)
airflow/contrib/operators/cloudformation_delete_stack_operator.py:45:57: E1101: Method '__init__' has no '__wrapped__' member (no-member)

turbaszek

Please move those operators to airflow.providers.amazon.aws.hooks/operators.cloud_formation.py. Additionally we try to keep operators for a single service in a single file.

turbaszek · 2019-12-18T14:26:09Z

airflow/contrib/hooks/aws_cloudformation_hook.py

+    Interact with AWS CloudFormation.
+    """
+
+    def __init__(self, region_name=None, *args, **kwargs):


Maybe it's worth to move region_name to methods? Then user will be able to specify this parameter using operators.

Was going by the example of other AWS hooks
airflow/contrib/hooks/emr_hook.py
airflow/contrib/hooks/aws_logs_hook.py
Should I still change this?

@feluelle I think you are more AWS experienced, can you take a look? 😃

turbaszek · 2019-12-18T14:28:46Z

airflow/contrib/operators/cloudformation_create_stack_operator.py

+
+    :param aws_conn_id: aws connection to uses
+    :type aws_conn_id: str
+    :param kwargs: Additional arguments to be passed to CloudFormation. For possible arguments see:


That's not a good idea. Please use other name for the argument like params or stack.

Sure, will use params instead.

turbaszek · 2019-12-18T14:29:10Z

airflow/contrib/operators/cloudformation_create_stack_operator.py

+            aws_conn_id='aws_default',
+            *args, **kwargs):
+        # pylint: disable=no-member
+        self.operator_arguments = inspect.getfullargspec(super().__init__.__wrapped__)[0]


Can you explain this?

Was trying to use kwargs for configuration.
As you mentioned above I should change this to use param instead, and will delete this code.

aviemzur · 2019-12-22T09:03:10Z

Sure, I can consolidate to one file.

I was going by the example of emr where there are 3 separate files
airflow/contrib/operators/emr_add_steps_operator.py
airflow/contrib/operators/emr_create_job_flow_operator.py
airflow/contrib/operators/emr_terminate_job_flow_operator.py

But have no problem to consolidate to one file.

Add 2 new operators: CloudFormationCreateStackOperator and CloudFormationDeleteStackOperator

potiuk · 2020-01-02T13:33:53Z

Please move those operators to airflow.providers.amazon.aws.hooks/operators.cloud_formation.py. Additionally we try to keep operators for a single service in a single file.

Just a nice reminder to do this as well :)

turbaszek · 2020-01-02T13:33:56Z

Please move those operators to airflow.providers.amazon.aws.hooks/operators.cloud_formation.py.

@aviemzur can you please add new operators to airflow/providers not to airflow/contrib? Contrib is going to be deprecated.

aviemzur · 2020-01-02T14:29:31Z

Sure, will move them

aviemzur · 2020-01-06T14:30:12Z

@nuclearpinguin made the requested change.
Not sure about how to fix the CI error:

/opt/airflow/docs/_api/airflow/providers/amazon/aws/operators/cloud_formation/index.rst:22: WARNING: Field list ends without a blank line; unexpected unindent.
/opt/airflow/docs/_api/airflow/providers/amazon/aws/operators/cloud_formation/index.rst:67: WARNING: Field list ends without a blank line; unexpected unindent.
/opt/airflow/docs/_api/airflow/providers/amazon/aws/operators/cloud_formation/index.rst:100: WARNING: Field list ends without a blank line; unexpected unindent.

Please advise

mik-laj · 2020-01-06T14:41:21Z

airflow/providers/amazon/aws/operators/cloud_formation.py

+    Base operator for CloudFormation operations.
+
+    :param params: parameters to be passed to CloudFormation.
+    :type dict


Suggested change

:type dict

:type param: Dict

mik-laj · 2020-01-06T14:42:10Z

airflow/providers/amazon/aws/operators/cloud_formation.py

+
+    :param params: parameters to be passed to CloudFormation. For possible arguments see:
+        https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/cloudformation.html#CloudFormation.Client.create_stack
+    :type dict


Suggested change

:type dict

:type params: Dict

mik-laj · 2020-01-06T14:42:23Z

airflow/providers/amazon/aws/operators/cloud_formation.py

+
+    :param params: parameters to be passed to CloudFormation. For possible arguments see:
+        https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/cloudformation.html#CloudFormation.Client.delete_stack
+    :type dict


Suggested change

:type dict

:type params: Dict

mik-laj · 2020-01-06T14:43:07Z

That should solve the problems.

codecov-io · 2020-01-06T15:44:15Z

Codecov Report

Merging #6824 into master will increase coverage by 0.35%.
The diff coverage is 99%.

@@            Coverage Diff            @@
##           master   #6824      +/-   ##
=========================================
+ Coverage   84.85%   85.2%   +0.35%     
=========================================
  Files         679     841     +162     
  Lines       38536   40424    +1888     
=========================================
+ Hits        32698   34445    +1747     
- Misses       5838    5979     +141

Impacted Files	Coverage Δ
.../providers/amazon/aws/operators/cloud_formation.py	`100% <100%> (ø)`
...ow/providers/amazon/aws/sensors/cloud_formation.py	`100% <100%> (ø)`
...flow/providers/amazon/aws/hooks/cloud_formation.py	`96.77% <96.77%> (ø)`
airflow/operators/postgres_operator.py	`0% <0%> (-100%)`	⬇️
...rflow/contrib/sensors/sagemaker_training_sensor.py	`0% <0%> (-100%)`	⬇️
airflow/contrib/operators/snowflake_operator.py	`0% <0%> (-95.84%)`	⬇️
airflow/contrib/hooks/azure_data_lake_hook.py	`0% <0%> (-93.11%)`	⬇️
airflow/contrib/hooks/grpc_hook.py	`0% <0%> (-91.94%)`	⬇️
airflow/contrib/sensors/azure_cosmos_sensor.py	`0% <0%> (-81.25%)`	⬇️
airflow/contrib/hooks/snowflake_hook.py	`0% <0%> (-81.14%)`	⬇️
... and 481 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be812bd...78a5d82. Read the comment docs.

aviemzur · 2020-01-22T11:47:08Z

@nuclearpinguin made the changes you requested: Changes after CR 3

airflow/providers/amazon/aws/operators/cloud_formation.py

feluelle · 2020-01-22T14:22:41Z

airflow/providers/amazon/aws/operators/cloud_formation.py

+        """
+        This is the main method to run CloudFormation operation.
+        """
+        raise NotImplementedError()


Agree with @nuclearpinguin

airflow/providers/amazon/aws/sensors/cloud_formation.py

airflow/providers/amazon/aws/operators/cloud_formation.py

airflow/providers/amazon/aws/hooks/cloud_formation.py

airflow/providers/amazon/aws/operators/cloud_formation.py

Co-Authored-By: Felix Uellendall <feluelle@users.noreply.github.com>

Change elif to if

aviemzur · 2020-01-27T15:35:03Z

@feluelle committed all suggestions you made + made the changes you requested and all tests are passing on Travis.

aviemzur · 2020-01-30T13:51:49Z

@nuclearpinguin @feluelle is anything else required prior to merging?

kaxil · 2020-02-02T09:27:49Z

Good work @aviemzur 🎉

…6824)

aviemzur force-pushed the AIRFLOW-6258-cloudformation-create-stack-delete-stack-operators branch from 175ab03 to 42e864c Compare December 17, 2019 12:19

turbaszek requested changes Dec 18, 2019

View reviewed changes

turbaszek added the provider:amazon AWS/Amazon - related issues label Dec 22, 2019

aviemzur force-pushed the AIRFLOW-6258-cloudformation-create-stack-delete-stack-operators branch 3 times, most recently from 661c4b3 to e0e6e99 Compare December 26, 2019 13:38

aviemzur added 2 commits January 2, 2020 13:31

[AIRFLOW-6258] add CloudFormation operators to contrib

f287f9d

Add 2 new operators: CloudFormationCreateStackOperator and CloudFormationDeleteStackOperator

Changes after CR apache#1

0814c9c

aviemzur force-pushed the AIRFLOW-6258-cloudformation-create-stack-delete-stack-operators branch 2 times, most recently from 5895b59 to 9924987 Compare January 2, 2020 11:56

CloudFormation sensor

51d2028

aviemzur force-pushed the AIRFLOW-6258-cloudformation-create-stack-delete-stack-operators branch from 9924987 to 51d2028 Compare January 2, 2020 12:33

aviemzur force-pushed the AIRFLOW-6258-cloudformation-create-stack-delete-stack-operators branch from cbe77b7 to 45e68c0 Compare January 3, 2020 04:44

aviemzur changed the title ~~[AIRFLOW-6258] add CloudFormation operators to contrib~~ [AIRFLOW-6258] add CloudFormation operators to aws providers Jan 3, 2020

aviemzur changed the title ~~[AIRFLOW-6258] add CloudFormation operators to aws providers~~ [AIRFLOW-6258] add CloudFormation operators to AWS providers Jan 3, 2020

Move files to providers module

b92339b

aviemzur force-pushed the AIRFLOW-6258-cloudformation-create-stack-delete-stack-operators branch from 45e68c0 to b92339b Compare January 3, 2020 14:22

github-actions bot added the area:docs label Jan 5, 2020

mik-laj reviewed Jan 6, 2020

View reviewed changes

Fix docs

a92c6c9

aviemzur force-pushed the AIRFLOW-6258-cloudformation-create-stack-delete-stack-operators branch from e8d9972 to 2af82f2 Compare January 8, 2020 12:32

aviemzur force-pushed the AIRFLOW-6258-cloudformation-create-stack-delete-stack-operators branch from 8b6a9c4 to fcd35bc Compare January 22, 2020 08:16

Changes after CR 3

3dce509

aviemzur force-pushed the AIRFLOW-6258-cloudformation-create-stack-delete-stack-operators branch from fcd35bc to 3dce509 Compare January 22, 2020 08:39

turbaszek approved these changes Jan 22, 2020

View reviewed changes

turbaszek requested review from feluelle and mik-laj January 22, 2020 12:09

feluelle reviewed Jan 22, 2020

View reviewed changes

aviemzur force-pushed the AIRFLOW-6258-cloudformation-create-stack-delete-stack-operators branch from eb9e8c6 to d81c4fc Compare January 27, 2020 09:06

Changes after CR 4

4147ffc

aviemzur force-pushed the AIRFLOW-6258-cloudformation-create-stack-delete-stack-operators branch from d81c4fc to 4147ffc Compare January 27, 2020 09:48

aviemzur closed this Jan 27, 2020

aviemzur reopened this Jan 27, 2020

feluelle requested changes Jan 27, 2020

View reviewed changes

aviemzur and others added 5 commits January 27, 2020 15:48

Update airflow/providers/amazon/aws/operators/cloud_formation.py

a477266

Co-Authored-By: Felix Uellendall <feluelle@users.noreply.github.com>

Update airflow/providers/amazon/aws/hooks/cloud_formation.py

c740f5d

Co-Authored-By: Felix Uellendall <feluelle@users.noreply.github.com>

Update airflow/providers/amazon/aws/hooks/cloud_formation.py

dfda9eb

Co-Authored-By: Felix Uellendall <feluelle@users.noreply.github.com>

Update airflow/providers/amazon/aws/operators/cloud_formation.py

eaa32b7

Co-Authored-By: Felix Uellendall <feluelle@users.noreply.github.com>

Changes after CR 5

78a5d82

Change elif to if

turbaszek requested a review from feluelle January 28, 2020 15:16

feluelle approved these changes Jan 30, 2020

View reviewed changes

kaxil approved these changes Feb 2, 2020

View reviewed changes

kaxil merged commit 63aa3db into apache:master Feb 2, 2020

aviemzur deleted the AIRFLOW-6258-cloudformation-create-stack-delete-stack-operators branch February 2, 2020 11:01

galuszkak pushed a commit to FlyrInc/apache-airflow that referenced this pull request Mar 5, 2020

[AIRFLOW-6258] Add CloudFormation operators to AWS providers (apache#…

5090f52

…6824)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[AIRFLOW-6258] add CloudFormation operators to AWS providers #6824

[AIRFLOW-6258] add CloudFormation operators to AWS providers #6824

aviemzur commented Dec 15, 2019 •

edited

Loading

OmerJog commented Dec 17, 2019

turbaszek left a comment

turbaszek Dec 18, 2019

aviemzur Dec 22, 2019

turbaszek Dec 22, 2019

turbaszek Dec 18, 2019

aviemzur Dec 22, 2019

turbaszek Dec 18, 2019

aviemzur Dec 22, 2019

aviemzur commented Dec 22, 2019

potiuk commented Jan 2, 2020

turbaszek commented Jan 2, 2020

aviemzur commented Jan 2, 2020

aviemzur commented Jan 6, 2020

mik-laj Jan 6, 2020 •

edited

Loading

mik-laj Jan 6, 2020

mik-laj Jan 6, 2020 •

edited

Loading

mik-laj commented Jan 6, 2020

codecov-io commented Jan 6, 2020 •

edited

Loading

aviemzur commented Jan 22, 2020

feluelle Jan 22, 2020

aviemzur commented Jan 27, 2020

aviemzur commented Jan 30, 2020

kaxil commented Feb 2, 2020

[AIRFLOW-6258] add CloudFormation operators to AWS providers #6824

[AIRFLOW-6258] add CloudFormation operators to AWS providers #6824

Conversation

aviemzur commented Dec 15, 2019 • edited Loading

Jira

Description

Tests

Commits

Documentation

OmerJog commented Dec 17, 2019

turbaszek left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aviemzur commented Dec 22, 2019

potiuk commented Jan 2, 2020

turbaszek commented Jan 2, 2020

aviemzur commented Jan 2, 2020

aviemzur commented Jan 6, 2020

mik-laj Jan 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mik-laj Jan 6, 2020 • edited Loading

Choose a reason for hiding this comment

mik-laj commented Jan 6, 2020

codecov-io commented Jan 6, 2020 • edited Loading

Codecov Report

aviemzur commented Jan 22, 2020

Choose a reason for hiding this comment

aviemzur commented Jan 27, 2020

aviemzur commented Jan 30, 2020

kaxil commented Feb 2, 2020

aviemzur commented Dec 15, 2019 •

edited

Loading

mik-laj Jan 6, 2020 •

edited

Loading

mik-laj Jan 6, 2020 •

edited

Loading

codecov-io commented Jan 6, 2020 •

edited

Loading