[GitHub] [airflow] mustafagok commented on a change in pull request #8888: Add AWS ECS system test

2020-06-08 Thread GitBox


mustafagok commented on a change in pull request #:
URL: https://github.com/apache/airflow/pull/#discussion_r436290678



##
File path: tests/providers/amazon/aws/operators/test_ecs_system.py
##
@@ -0,0 +1,99 @@
+#
+# 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.
+#
+
+import pytest
+
+from tests.test_utils.amazon_system_helpers import AWS_DAG_FOLDER, 
AmazonSystemTest
+
+
+@pytest.mark.system("amazon")
+class ECSSystemTest(AmazonSystemTest):
+"""
+ECS System Test to run and test example ECS dags
+
+Required variables.env file content (from your account):
+# Auto-export all variables
+set -a
+
+# aws parameters
+REGION_NAME="eu-west-1"
+REGISTRY_ID="123456789012"
+IMAGE="alpine:3.9"
+SUBNET_ID="subnet-068e9654a3c357a"
+SECURITY_GROUP_ID="sg-054dc69874a651"
+EXECUTION_ROLE_ARN="arn:aws:iam::123456789012:role/FooBarRole"
+
+# remove all created/existing resources flag
+# comment out to keep resources or use empty string
+# REMOVE_RESOURCES="True"
+"""
+
+# should be same as in the example dag
+aws_conn_id = "aws_ecs"
+cluster = "c"
+task_definition = "hello-world"
+container = "hello-world-container"
+awslogs_group = "/ecs/hello-world"
+awslogs_stream_prefix = "prefix_b"  # only prefix without container name
+
+def setUp(self):

Review comment:
   done, can you check it please?

##
File path: tests/providers/amazon/aws/operators/test_ecs_system.py
##
@@ -0,0 +1,96 @@
+#
+# 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.
+#
+
+from tests.test_utils.amazon_system_helpers import AWS_DAG_FOLDER, 
AmazonSystemTest
+
+

Review comment:
   you are right it should work but I didn't test it with `sqlite`
   
   :heavy_check_mark: done, I have added mark-backend

##
File path: tests/providers/amazon/aws/operators/test_ecs_system.py
##
@@ -0,0 +1,99 @@
+#
+# 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.
+#
+
+import pytest
+
+from tests.test_utils.amazon_system_helpers import AWS_DAG_FOLDER, 
AmazonSystemTest
+
+
+@pytest.mark.system("amazon")
+class ECSSystemTest(AmazonSystemTest):
+"""
+ECS System Test to run and test example ECS dags
+
+Required variables.env file content (from your account):
+# Auto-export all variables
+set -a
+
+# aws parameters
+REGION_NAME="eu-west-1"
+REGISTRY_ID="123456789012"
+IMAGE="alpine:3.9"
+SUBNET_ID="subnet-068e9654a3c357a"
+SECURITY_GROUP_ID="sg-054dc69874a651"
+EXECUTION_ROLE_ARN="arn:aws:iam::123456789012:role/FooBarRole"
+
+# remove all created/existing resources flag
+ 

[GitHub] [airflow] mustafagok commented on a change in pull request #8888: Add AWS ECS system test

2020-06-06 Thread GitBox


mustafagok commented on a change in pull request #:
URL: https://github.com/apache/airflow/pull/#discussion_r436306671



##
File path: tests/providers/amazon/aws/operators/test_ecs_system.py
##
@@ -0,0 +1,99 @@
+#
+# 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.
+#
+
+import pytest
+
+from tests.test_utils.amazon_system_helpers import AWS_DAG_FOLDER, 
AmazonSystemTest
+
+
+@pytest.mark.system("amazon")
+class ECSSystemTest(AmazonSystemTest):
+"""
+ECS System Test to run and test example ECS dags
+
+Required variables.env file content (from your account):
+# Auto-export all variables
+set -a
+
+# aws parameters
+REGION_NAME="eu-west-1"
+REGISTRY_ID="123456789012"
+IMAGE="alpine:3.9"
+SUBNET_ID="subnet-068e9654a3c357a"
+SECURITY_GROUP_ID="sg-054dc69874a651"
+EXECUTION_ROLE_ARN="arn:aws:iam::123456789012:role/FooBarRole"
+
+# remove all created/existing resources flag
+# comment out to keep resources or use empty string
+# REMOVE_RESOURCES="True"
+"""
+
+# should be same as in the example dag
+aws_conn_id = "aws_ecs"
+cluster = "c"
+task_definition = "hello-world"
+container = "hello-world-container"
+awslogs_group = "/ecs/hello-world"
+awslogs_stream_prefix = "prefix_b"  # only prefix without container name
+
+def setUp(self):

Review comment:
   thank you for your review





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [airflow] mustafagok commented on a change in pull request #8888: Add AWS ECS system test

2020-06-06 Thread GitBox


mustafagok commented on a change in pull request #:
URL: https://github.com/apache/airflow/pull/#discussion_r436291008



##
File path: tests/providers/amazon/aws/operators/test_ecs_system.py
##
@@ -0,0 +1,96 @@
+#
+# 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.
+#
+
+from tests.test_utils.amazon_system_helpers import AWS_DAG_FOLDER, 
AmazonSystemTest
+
+

Review comment:
   you are right it should work but I didn't test it with `sqlite`
   
   :heavy_check_mark: done, I have added mark-backend





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [airflow] mustafagok commented on a change in pull request #8888: Add AWS ECS system test

2020-06-06 Thread GitBox


mustafagok commented on a change in pull request #:
URL: https://github.com/apache/airflow/pull/#discussion_r436290678



##
File path: tests/providers/amazon/aws/operators/test_ecs_system.py
##
@@ -0,0 +1,99 @@
+#
+# 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.
+#
+
+import pytest
+
+from tests.test_utils.amazon_system_helpers import AWS_DAG_FOLDER, 
AmazonSystemTest
+
+
+@pytest.mark.system("amazon")
+class ECSSystemTest(AmazonSystemTest):
+"""
+ECS System Test to run and test example ECS dags
+
+Required variables.env file content (from your account):
+# Auto-export all variables
+set -a
+
+# aws parameters
+REGION_NAME="eu-west-1"
+REGISTRY_ID="123456789012"
+IMAGE="alpine:3.9"
+SUBNET_ID="subnet-068e9654a3c357a"
+SECURITY_GROUP_ID="sg-054dc69874a651"
+EXECUTION_ROLE_ARN="arn:aws:iam::123456789012:role/FooBarRole"
+
+# remove all created/existing resources flag
+# comment out to keep resources or use empty string
+# REMOVE_RESOURCES="True"
+"""
+
+# should be same as in the example dag
+aws_conn_id = "aws_ecs"
+cluster = "c"
+task_definition = "hello-world"
+container = "hello-world-container"
+awslogs_group = "/ecs/hello-world"
+awslogs_stream_prefix = "prefix_b"  # only prefix without container name
+
+def setUp(self):

Review comment:
   done, can you check it please?





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [airflow] mustafagok commented on a change in pull request #8888: Add AWS ECS system test

2020-05-31 Thread GitBox


mustafagok commented on a change in pull request #:
URL: https://github.com/apache/airflow/pull/#discussion_r432989031



##
File path: tests/providers/amazon/aws/operators/test_ecs_system.py
##
@@ -0,0 +1,99 @@
+#
+# 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.
+#
+
+import pytest
+
+from tests.test_utils.amazon_system_helpers import AWS_DAG_FOLDER, 
AmazonSystemTest
+
+
+@pytest.mark.system("amazon")
+class ECSSystemTest(AmazonSystemTest):
+"""
+ECS System Test to run and test example ECS dags
+
+Required variables.env file content (from your account):
+# Auto-export all variables
+set -a
+
+# aws parameters
+REGION_NAME="eu-west-1"
+REGISTRY_ID="123456789012"
+IMAGE="alpine:3.9"
+SUBNET_ID="subnet-068e9654a3c357a"
+SECURITY_GROUP_ID="sg-054dc69874a651"
+EXECUTION_ROLE_ARN="arn:aws:iam::123456789012:role/FooBarRole"
+
+# remove all created/existing resources flag
+# comment out to keep resources or use empty string
+# REMOVE_RESOURCES="True"
+"""
+
+# should be same as in the example dag
+aws_conn_id = "aws_ecs"
+cluster = "c"
+task_definition = "hello-world"
+container = "hello-world-container"
+awslogs_group = "/ecs/hello-world"
+awslogs_stream_prefix = "prefix_b"  # only prefix without container name
+
+def setUp(self):

Review comment:
   is it `setup_class` or `setUpClass`? see 
[docs](https://docs.python.org/3/library/unittest.html#unittest.TestCase.setUpClass)





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [airflow] mustafagok commented on a change in pull request #8888: Add AWS ECS system test

2020-05-29 Thread GitBox


mustafagok commented on a change in pull request #:
URL: https://github.com/apache/airflow/pull/#discussion_r432724181



##
File path: tests/providers/amazon/aws/operators/test_ecs_system.py
##
@@ -0,0 +1,99 @@
+#
+# 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.
+#
+
+import pytest
+
+from tests.test_utils.amazon_system_helpers import AWS_DAG_FOLDER, 
AmazonSystemTest
+
+
+@pytest.mark.system("amazon")
+class ECSSystemTest(AmazonSystemTest):
+"""
+ECS System Test to run and test example ECS dags
+
+Required variables.env file content (from your account):
+# Auto-export all variables
+set -a
+
+# aws parameters
+REGION_NAME="eu-west-1"
+REGISTRY_ID="123456789012"
+IMAGE="alpine:3.9"
+SUBNET_ID="subnet-068e9654a3c357a"
+SECURITY_GROUP_ID="sg-054dc69874a651"
+EXECUTION_ROLE_ARN="arn:aws:iam::123456789012:role/FooBarRole"
+
+# remove all created/existing resources flag
+# comment out to keep resources or use empty string
+# REMOVE_RESOURCES="True"
+"""
+
+# should be same as in the example dag
+aws_conn_id = "aws_ecs"
+cluster = "c"
+task_definition = "hello-world"
+container = "hello-world-container"
+awslogs_group = "/ecs/hello-world"
+awslogs_stream_prefix = "prefix_b"  # only prefix without container name
+
+def setUp(self):

Review comment:
   @xinbinhuang actually I don't understand what you mean





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [airflow] mustafagok commented on a change in pull request #8888: Add AWS ECS system test

2020-05-29 Thread GitBox


mustafagok commented on a change in pull request #:
URL: https://github.com/apache/airflow/pull/#discussion_r432721671



##
File path: tests/test_utils/amazon_system_helpers.py
##
@@ -60,6 +84,25 @@ def execute_with_ctx(cls, cmd: List[str]):
 with provide_aws_context():
 executor.execute_cmd(cmd=cmd)
 
+@staticmethod
+def create_connection(aws_conn_id: str,
+  region: str = "eu-west-1") -> None:

Review comment:
   done :heavy_check_mark:  thanks





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [airflow] mustafagok commented on a change in pull request #8888: Add AWS ECS system test

2020-05-23 Thread GitBox


mustafagok commented on a change in pull request #:
URL: https://github.com/apache/airflow/pull/#discussion_r429561330



##
File path: tests/providers/amazon/aws/operators/test_ecs_system.py
##
@@ -0,0 +1,140 @@
+#
+# 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.
+#
+
+import pytest
+
+from tests.test_utils.amazon_system_helpers import AmazonSystemTest, 
AWS_DAG_FOLDER
+
+
+@pytest.mark.system("amazon")
+class ECSSystemTest(AmazonSystemTest):
+"""
+ECS System Test to run and test example ECS dags
+
+Required variables.env file content (from your account):
+# Auto-export all variables
+set -a
+
+# aws parameters
+REGION_NAME="eu-west-1"
+REGISTRY_ID="123456789012"
+REPOSITORY_NAME="foobar/hello-world"
+SUBNET_ID="subnet-068e9654a3c357a"
+SECURITY_GROUP_ID="sg-054dc69874a651"
+EXECUTION_ROLE_ARN="arn:aws:iam::123456789012:role/FooBarRole"
+
+# remove all created/existing resources flag
+# comment out to keep resources or use empty string
+# REMOVE_RESOURCES="True"
+"""
+
+# should be same as in the example dag
+aws_conn_id = "aws_ecs"
+cluster = "c"
+task_definition = "hello-world"
+container = "hello-world-container"
+awslogs_group = "/ecs/hello-world"
+awslogs_stream_prefix = "prefix_b/hello-world-container"
+
+local_image_tag = "hello"
+local_image_context_path = 
"/opt/airflow/tests/providers/amazon/aws/images/ecs_test_image/"
+
+def setUp(self):
+print("setUp")
+super().setUp()
+self.create_connection(
+aws_conn_id=self.aws_conn_id,
+region=self._region_name(),
+)
+
+# create repository in ecr if it does not exist
+repository_exists, repository_uri = self.is_ecr_repository_exists(
+aws_conn_id=self.aws_conn_id,
+repository_name=self._repository_name(),
+registry_id=self._registry_id(),
+)
+print(f"repository_exists: {repository_exists}")
+if not repository_exists:
+repository_uri = self.create_ecr_repository(
+aws_conn_id=self.aws_conn_id,
+repository_name=self._repository_name(),
+)
+print(f"repository_uri: {repository_uri}")
+full_image_name = f"{repository_uri}:latest"
+
+# prepare (build, tag, push) image
+self.authenticate_client_to_ecr(
+region=self._region_name(),
+)
+self.build_image(
+tag=self.local_image_tag,
+path=self.local_image_context_path,
+)
+self.tag_image(
+source=f"{self.local_image_tag}:latest",
+target=full_image_name,
+)
+self.push_image(
+tag=full_image_name,
+)

Review comment:
   done, thanks





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [airflow] mustafagok commented on a change in pull request #8888: Add AWS ECS system test

2020-05-23 Thread GitBox


mustafagok commented on a change in pull request #:
URL: https://github.com/apache/airflow/pull/#discussion_r429554430



##
File path: tests/test_utils/amazon_system_helpers.py
##
@@ -51,6 +54,27 @@ def provide_aws_s3_bucket(name):
 @pytest.mark.system("amazon")
 class AmazonSystemTest(SystemTest):
 
+@staticmethod
+def _region_name():
+return os.environ.get("REGION_NAME")
+
+@staticmethod
+def _registry_id():
+return os.environ.get("REGISTRY_ID")
+
+@staticmethod
+def _repository_name():
+return os.environ.get("REPOSITORY_NAME")
+
+@staticmethod
+def _execution_role_arn():
+return os.environ.get("EXECUTION_ROLE_ARN")

Review comment:
   resolving this thread, reopen if you want me to change please. thank you 
all for review





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [airflow] mustafagok commented on a change in pull request #8888: Add AWS ECS system test

2020-05-23 Thread GitBox


mustafagok commented on a change in pull request #:
URL: https://github.com/apache/airflow/pull/#discussion_r429554235



##
File path: tests/test_utils/amazon_system_helpers.py
##
@@ -60,6 +84,25 @@ def execute_with_ctx(cls, cmd: List[str]):
 with provide_aws_context():
 executor.execute_cmd(cmd=cmd)
 
+@staticmethod
+def create_connection(aws_conn_id: str,
+  region: str = "eu-west-1") -> None:

Review comment:
   I defined default values but not used, user have to define environment 
variable. You can check PR description and the docstring of ECSSystemTest 
class. also line 62, 80 and 113 in the 
airflow/tests/providers/amazon/aws/operators/test_ecs_system.py file.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [airflow] mustafagok commented on a change in pull request #8888: Add AWS ECS system test

2020-05-23 Thread GitBox


mustafagok commented on a change in pull request #:
URL: https://github.com/apache/airflow/pull/#discussion_r429553900



##
File path: airflow/providers/amazon/aws/example_dags/example_ecs_fargate.py
##
@@ -64,8 +65,8 @@
 },
 network_configuration={
 "awsvpcConfiguration": {
-"securityGroups": ["sg-123abc"],
-"subnets": ["subnet-123456ab"],
+"securityGroups": [os.environ.get("SECURITY_GROUP_ID")],  # or 
simply ["sg-123abc"],
+"subnets": [os.environ.get("SUBNET_ID")],  # or simply 
["subnet-123456ab"],

Review comment:
   these are example (placeholder) values, because this is example dag. I 
just want to show that the reader can use hard-coded values. Also, at the same 
time the example dag can be used in system tests. I can change as you suggest 
but the "fallback values" are not going to work.





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [airflow] mustafagok commented on a change in pull request #8888: Add AWS ECS system test

2020-05-16 Thread GitBox


mustafagok commented on a change in pull request #:
URL: https://github.com/apache/airflow/pull/#discussion_r426208265



##
File path: tests/providers/amazon/aws/operators/test_ecs_system.py
##
@@ -0,0 +1,140 @@
+#
+# 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.
+#
+
+import pytest
+
+from tests.test_utils.amazon_system_helpers import AmazonSystemTest, 
AWS_DAG_FOLDER
+
+
+@pytest.mark.system("amazon")
+class ECSSystemTest(AmazonSystemTest):
+"""
+ECS System Test to run and test example ECS dags
+
+Required variables.env file content (from your account):
+# Auto-export all variables
+set -a
+
+# aws parameters
+REGION_NAME="eu-west-1"
+REGISTRY_ID="123456789012"
+REPOSITORY_NAME="foobar/hello-world"
+SUBNET_ID="subnet-068e9654a3c357a"
+SECURITY_GROUP_ID="sg-054dc69874a651"
+EXECUTION_ROLE_ARN="arn:aws:iam::123456789012:role/FooBarRole"
+
+# remove all created/existing resources flag
+# comment out to keep resources or use empty string
+# REMOVE_RESOURCES="True"
+"""
+
+# should be same as in the example dag
+aws_conn_id = "aws_ecs"
+cluster = "c"
+task_definition = "hello-world"
+container = "hello-world-container"
+awslogs_group = "/ecs/hello-world"
+awslogs_stream_prefix = "prefix_b/hello-world-container"
+
+local_image_tag = "hello"
+local_image_context_path = 
"/opt/airflow/tests/providers/amazon/aws/images/ecs_test_image/"
+
+def setUp(self):
+print("setUp")
+super().setUp()
+self.create_connection(
+aws_conn_id=self.aws_conn_id,
+region=self._region_name(),
+)
+
+# create repository in ecr if it does not exist
+repository_exists, repository_uri = self.is_ecr_repository_exists(
+aws_conn_id=self.aws_conn_id,
+repository_name=self._repository_name(),
+registry_id=self._registry_id(),
+)
+print(f"repository_exists: {repository_exists}")
+if not repository_exists:
+repository_uri = self.create_ecr_repository(
+aws_conn_id=self.aws_conn_id,
+repository_name=self._repository_name(),
+)
+print(f"repository_uri: {repository_uri}")
+full_image_name = f"{repository_uri}:latest"
+
+# prepare (build, tag, push) image
+self.authenticate_client_to_ecr(
+region=self._region_name(),
+)
+self.build_image(
+tag=self.local_image_tag,
+path=self.local_image_context_path,
+)
+self.tag_image(
+source=f"{self.local_image_tag}:latest",
+target=full_image_name,
+)
+self.push_image(
+tag=full_image_name,
+)

Review comment:
   probably we can, I don't know, never used.
   
   we use ECS with ECR in the company I work for. It is closer example/test for 
`ECSOperator` :smile:
   
   building/pushing image is pretty fast when compared to AWS starting a 
container on ECS





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [airflow] mustafagok commented on a change in pull request #8888: Add AWS ECS system test

2020-05-16 Thread GitBox


mustafagok commented on a change in pull request #:
URL: https://github.com/apache/airflow/pull/#discussion_r426207757



##
File path: tests/test_utils/amazon_system_helpers.py
##
@@ -92,3 +135,310 @@ def create_emr_default_roles(cls) -> None:
 """
 cmd = ["aws", "emr", "create-default-roles"]
 cls.execute_with_ctx(cmd)
+
+@staticmethod
+def create_ecr_repository(aws_conn_id: str,
+  repository_name: str) -> str:
+"""
+Create repository in ecr with given name
+
+:param aws_conn_id: id of the aws connection to use when creating 
boto3 client/resource
+:type aws_conn_id: str
+:param repository_name: name of the repository to create in aws ecr
+:type repository_name: str
+:return: uri of the created repository
+uri format: 
`registry_id`.dkr.ecr.`region`.amazonaws.com/`repository_name`
+:rtype: str
+"""
+hook = AwsBaseHook(
+aws_conn_id=aws_conn_id,
+client_type="ecr",
+)
+response = hook.conn.create_repository(
+repositoryName=repository_name,
+imageTagMutability="MUTABLE",
+imageScanningConfiguration={
+"scanOnPush": False
+},
+)
+return response["repository"]["repositoryUri"]
+
+@staticmethod
+def delete_ecr_repository(aws_conn_id: str,
+  repository_name: str,
+  registry_id: str) -> None:
+"""
+Delete repository in ecr with given name
+
+:param aws_conn_id: id of the aws connection to use when creating 
boto3 client/resource
+:type aws_conn_id: str
+:param repository_name: name of the repository to delete in aws ecr
+:type repository_name: str
+:param registry_id: aws account id associated with the registry that 
contains the repository to delete
+:type registry_id: str
+"""
+hook = AwsBaseHook(
+aws_conn_id=aws_conn_id,
+client_type="ecr",
+)
+hook.conn.delete_repository(
+registryId=registry_id,
+repositoryName=repository_name,
+force=True,  # also delete images
+)
+
+@staticmethod
+def is_ecr_repository_exists(aws_conn_id: str,
+ repository_name: str,
+ registry_id: str) -> Tuple[bool, str]:
+"""
+Check whether given repository exists in ecr
+
+:param aws_conn_id: id of the aws connection to use when creating 
boto3 client/resource
+:type aws_conn_id: str
+:param repository_name: name of the repository to check in aws ecr
+:type repository_name: str
+:param registry_id: aws account id associated with the registry that 
contains the repository to delete
+:type registry_id: str
+:return: True and uri of the found repository if given repository 
exists in ecr,
+else False and empty string
+:rtype: Tuple[bool, str]
+"""
+hook = AwsBaseHook(
+aws_conn_id=aws_conn_id,
+client_type="ecr",
+)
+
+try:
+response = hook.conn.describe_repositories(
+registryId=registry_id,
+repositoryNames=[repository_name],
+)
+except hook.conn.exceptions.RepositoryNotFoundException:
+# return False if repository does not exist
+return False, ""
+
+# return True if repository exists
+return True, response["repositories"][0]["repositoryUri"]

Review comment:
   good point, thank you. I will change it.
   
   at first it was just returning True or False, then I added uri :sweat_smile: 





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org




[GitHub] [airflow] mustafagok commented on a change in pull request #8888: Add AWS ECS system test

2020-05-16 Thread GitBox


mustafagok commented on a change in pull request #:
URL: https://github.com/apache/airflow/pull/#discussion_r426207661



##
File path: tests/test_utils/amazon_system_helpers.py
##
@@ -51,6 +54,27 @@ def provide_aws_s3_bucket(name):
 @pytest.mark.system("amazon")
 class AmazonSystemTest(SystemTest):
 
+@staticmethod
+def _region_name():
+return os.environ.get("REGION_NAME")
+
+@staticmethod
+def _registry_id():
+return os.environ.get("REGISTRY_ID")
+
+@staticmethod
+def _repository_name():
+return os.environ.get("REPOSITORY_NAME")
+
+@staticmethod
+def _execution_role_arn():
+return os.environ.get("EXECUTION_ROLE_ARN")

Review comment:
   not sure, these are not getter and not returning `self._x`. the 
`@property` decorator is used for such things afaik.
   
   also `GoogleSystemTest` class has same usage





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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org