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 2020/08/26 09:37:40 UTC
[GitHub] [airflow] feluelle commented on a change in pull request #9011: Changing from Resource to Client for EC2 and adding support for filters
feluelle commented on a change in pull request #9011:
URL: https://github.com/apache/airflow/pull/9011#discussion_r477165215
##########
File path: airflow/providers/amazon/aws/hooks/ec2.py
##########
@@ -33,21 +33,90 @@ class EC2Hook(AwsBaseHook):
:class:`~airflow.providers.amazon.aws.hooks.base_aws.AwsBaseHook`
"""
+ # Describe response
+ RESERVATIONS = 'Reservations'
+ INSTANCES = 'Instances'
+ STATE = 'State'
+ NAME = 'Name'
+ INSTANCE_ID = 'InstanceId'
+
def __init__(self,
*args,
**kwargs):
- super().__init__(resource_type="ec2", *args, **kwargs)
+ super().__init__(client_type="ec2", *args, **kwargs)
- def get_instance(self, instance_id: str):
+ def stop_instances(self, instance_ids):
Review comment:
Please add type hints to all args and return values of your functions.
See https://docs.python.org/3/library/typing.html for more information.
##########
File path: airflow/providers/amazon/aws/hooks/ec2.py
##########
@@ -33,21 +33,90 @@ class EC2Hook(AwsBaseHook):
:class:`~airflow.providers.amazon.aws.hooks.base_aws.AwsBaseHook`
"""
+ # Describe response
+ RESERVATIONS = 'Reservations'
+ INSTANCES = 'Instances'
+ STATE = 'State'
+ NAME = 'Name'
+ INSTANCE_ID = 'InstanceId'
Review comment:
@VijayantSoni in the example you posted those values are outside of the hook. Those are specific _types_.
I agree with Ash on using the strings directly (in your case) as those doesn't really have a category to specify them under, do they?
In the example the category is JobTypes and JobStatus.
##########
File path: airflow/providers/amazon/aws/hooks/ec2.py
##########
@@ -33,21 +33,90 @@ class EC2Hook(AwsBaseHook):
:class:`~airflow.providers.amazon.aws.hooks.base_aws.AwsBaseHook`
"""
+ # Describe response
+ RESERVATIONS = 'Reservations'
+ INSTANCES = 'Instances'
+ STATE = 'State'
+ NAME = 'Name'
+ INSTANCE_ID = 'InstanceId'
+
def __init__(self,
*args,
**kwargs):
- super().__init__(resource_type="ec2", *args, **kwargs)
+ super().__init__(client_type="ec2", *args, **kwargs)
- def get_instance(self, instance_id: str):
+ def stop_instances(self, instance_ids):
"""
- Get EC2 instance by id and return it.
+ Stop instances with given ids
- :param instance_id: id of the AWS EC2 instance
- :type instance_id: str
- :return: Instance object
- :rtype: ec2.Instance
+ :param instance_ids: List of instance ids to stop
+ :return: Dict with key `StoppingInstances` and value as list of instances being stopped
+ """
+ self.log.info("Stopping instances: %s", instance_ids)
+
+ return self.conn.stop_instances(InstanceIds=instance_ids)
+
+ def start_instances(self, instance_ids):
+ """
+ Start instances with given ids
+
+ :param instance_ids: List of instance ids to start
+ :return: Dict with key `StartingInstances` and value as list of instances being started
+ """
+ self.log.info("Starting instances: %s", instance_ids)
+
+ return self.conn.start_instances(InstanceIds=instance_ids)
+
+ def terminate_instances(self, instance_ids):
+ """
+ Terminate instances with given ids
+
+ :param instance_ids: List of instance ids to terminate
+ :return: Dict with key `TerminatingInstances` and value as list of instances being terminated
+ """
+ self.log.info("Terminating instances: %s", instance_ids)
+
+ return self.conn.terminate_instances(InstanceIds=instance_ids)
+
+ def describe_instances(self, filters=None, instance_ids=None):
+ """
+ Describe EC2 instances, optionally applying filters and selective instance ids
+
+ :param filters: List of filters to specify instances to describe
+ :param instance_ids: List of instance IDs to describe
+ :return: Response from EC2 describe_instances API
+ """
+ filters = [] if filters is None else filters
+ instance_ids = [] if instance_ids is None else instance_ids
Review comment:
```suggestion
filters = filters or []
instance_ids = instance_ids or []
```
See https://realpython.com/python-or-operator/#using-or-with-common-objects
----------------------------------------------------------------
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:
users@infra.apache.org