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 2022/08/27 06:50:13 UTC
[GitHub] [airflow] hankehly opened a new pull request, #26003: Add RdsInstanceSensor to amazon provider package
hankehly opened a new pull request, #26003:
URL: https://github.com/apache/airflow/pull/26003
Related: #25952
### Summary
This PR adds the `RdsInstanceSensor` to the amazon provider package. It waits for an RDS instance to reach one (or more) of the DB instance states [described here](https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/accessing-monitoring.html).
### Todo
- [x] Add `RdsInstanceSensor`
- [x] Add/run unit tests
```
breeze shell
pytest tests/providers/amazon/aws/sensors/test_rds.py::TestRdsInstanceSensor
```
- [x] Add/run system tests
```
breeze shell
export AWS_ACCESS_KEY=***
export AWS_SECRET_ACCESS_KEY=***
airflow dags test -S tests/system/providers/amazon/aws/rds/example_rds_instance.py example_rds_instance 2022-08-01
```
- [x] Update documentation
--
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
[GitHub] [airflow] o-nikolas commented on pull request #26003: Add RdsInstanceSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
o-nikolas commented on PR #26003:
URL: https://github.com/apache/airflow/pull/26003#issuecomment-1234942165
> @o-nikolas @vincbeck @ferruzzi @potiuk Could someone please merge this PR, or let me know what else needs to change
Unfortunately none of us tagged above have the powers to merge code except for @potiuk
--
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
[GitHub] [airflow] ferruzzi commented on pull request #26003: Add RdsInstanceSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
ferruzzi commented on PR #26003:
URL: https://github.com/apache/airflow/pull/26003#issuecomment-1231007354
My concerns have been addressed, thanks. LGTM
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r962435821
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -51,6 +50,12 @@ def _describe_item(self, item_type: str, item_name: str) -> list:
elif item_type == 'export_task':
exports = self.hook.conn.describe_export_tasks(ExportTaskIdentifier=item_name)
return exports['ExportTasks']
+ elif item_type == "db_instance":
+ instances = self.hook.conn.describe_db_instances(DBInstanceIdentifier=item_name)
+ return instances["DBInstances"]
+ elif item_type == "db_cluster":
Review Comment:
2022/09/05 update
Added `db_cluster` to be consistent with https://github.com/apache/airflow/pull/21231 / https://github.com/apache/airflow/pull/20907
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -61,7 +66,14 @@ def _check_item(self, item_type: str, item_name: str) -> bool:
except ClientError:
return False
else:
- return bool(items) and any(map(lambda s: items[0]['Status'].lower() == s, self.target_statuses))
+ status_field = self._check_status_field()
Review Comment:
2022/09/05 update
Replaced instance variable with instance method to encapsulate if/else statement
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r962437745
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -149,7 +161,53 @@ def poke(self, context: 'Context'):
return self._check_item(item_type='export_task', item_name=self.export_task_identifier)
+class RdsDbSensor(RdsBaseSensor):
+ """
+ Waits for an RDS instance or cluster to enter one of a number of states
+
+ .. seealso::
+ For more information on how to use this sensor, take a look at the guide:
+ :ref:`howto/sensor:RdsDbSensor`
+
+ :param db_type: Type of the DB - either "instance" or "cluster"
+ :param db_identifier: The AWS identifier for the DB
+ :param target_statuses: Target status of DB
+ """
+
+ def __init__(
+ self,
+ *,
+ db_identifier: str,
+ db_type: str = RdsDbType.INSTANCE,
+ target_statuses: Optional[List[str]] = None,
+ aws_conn_id: str = "aws_default",
+ **kwargs,
+ ):
+ super().__init__(aws_conn_id=aws_conn_id, **kwargs)
+ self.db_identifier = db_identifier
+ self.target_statuses = target_statuses or ["available"]
+ self.db_type = RdsDbType(db_type)
+
+ def poke(self, context: 'Context'):
+ self.log.info(
+ "Poking for statuses : %s\nfor db instance %s", self.target_statuses, self.db_identifier
+ )
+ item_type = self._check_item_type()
+ return self._check_item(item_type=item_type, item_name=self.db_identifier)
+
+ def _check_status_field(self) -> str:
+ if self.db_type == RdsDbType.INSTANCE:
Review Comment:
2022/09/05 update (https://github.com/apache/airflow/pull/26003/commits/0d537b1bdf1fa3dbd8a94f1b635a5814e84e11e9)
Added `_check_status_field` and `_check_item_type` instance methods to account for different db types.
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r962435821
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -51,6 +50,12 @@ def _describe_item(self, item_type: str, item_name: str) -> list:
elif item_type == 'export_task':
exports = self.hook.conn.describe_export_tasks(ExportTaskIdentifier=item_name)
return exports['ExportTasks']
+ elif item_type == "db_instance":
+ instances = self.hook.conn.describe_db_instances(DBInstanceIdentifier=item_name)
+ return instances["DBInstances"]
+ elif item_type == "db_cluster":
Review Comment:
2022/09/05 update (https://github.com/apache/airflow/pull/26003/commits/0d537b1bdf1fa3dbd8a94f1b635a5814e84e11e9)
Added `db_cluster` to be consistent with https://github.com/apache/airflow/pull/21231 / https://github.com/apache/airflow/pull/20907
--
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
[GitHub] [airflow] hankehly commented on pull request #26003: Add RdsInstanceSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on PR #26003:
URL: https://github.com/apache/airflow/pull/26003#issuecomment-1236443949
Sorry everyone, to stay consistent with https://github.com/apache/airflow/pull/21231 / https://github.com/apache/airflow/pull/20907 I'm going to add the `db_type` argument to this class, and rename it to `RdsDbSensor`.
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r962437210
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -149,7 +161,54 @@ def poke(self, context: 'Context'):
return self._check_item(item_type='export_task', item_name=self.export_task_identifier)
+class RdsDbSensor(RdsBaseSensor):
Review Comment:
2022/09/05 update (https://github.com/apache/airflow/pull/26003/commits/0d537b1bdf1fa3dbd8a94f1b635a5814e84e11e9)
Renamed `RdsInstanceSensor` to `RdsDbSensor` because it considers "cluster" databases as well
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -149,7 +161,54 @@ def poke(self, context: 'Context'):
return self._check_item(item_type='export_task', item_name=self.export_task_identifier)
+class RdsDbSensor(RdsBaseSensor):
+ """
+ Waits for an RDS instance or cluster to enter one of a number of states
+
+ .. seealso::
+ For more information on how to use this sensor, take a look at the guide:
+ :ref:`howto/sensor:RdsDbSensor`
+
+ :param db_type: Type of the DB - either "instance" or "cluster"
+ :param db_identifier: The AWS identifier for the DB
+ :param target_statuses: Target status of DB
+ """
+
+ def __init__(
+ self,
+ *,
+ db_identifier: str,
+ db_type: str = RdsDbType.INSTANCE,
Review Comment:
2022/09/05 update (https://github.com/apache/airflow/pull/26003/commits/0d537b1bdf1fa3dbd8a94f1b635a5814e84e11e9)
Added `db_type`
--
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
[GitHub] [airflow] hankehly commented on pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on PR #26003:
URL: https://github.com/apache/airflow/pull/26003#issuecomment-1237670125
@o-nikolas @ferruzzi @vincbeck @kazanzhy
I renamed `RdsInstanceSensor` to `RdsDbSensor` and added the `db_type` parameter to match other sensor classes.
I left comments where the code changed.
Please re-OK the changes at your earliest convenience.
--
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
[GitHub] [airflow] ferruzzi commented on a diff in pull request #26003: Add RdsInstanceSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
ferruzzi commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r957900132
##########
tests/providers/amazon/aws/sensors/test_rds.py:
##########
@@ -50,15 +51,18 @@
EXPORT_TASK_SOURCE = 'arn:aws:rds:es-east-1::snapshot:my-db-instance-snap'
-def _create_db_instance_snapshot(hook: RdsHook):
+def _create_db_instance(hook: RdsHook):
hook.conn.create_db_instance(
DBInstanceIdentifier=DB_INSTANCE_NAME,
- DBInstanceClass='db.m4.large',
- Engine='postgres',
+ DBInstanceClass="db.t4g.micro",
Review Comment:
Alright, then I'm cool with the change. Thanks for checking that.
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsInstanceSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r957892008
##########
tests/providers/amazon/aws/sensors/test_rds.py:
##########
@@ -50,15 +51,18 @@
EXPORT_TASK_SOURCE = 'arn:aws:rds:es-east-1::snapshot:my-db-instance-snap'
-def _create_db_instance_snapshot(hook: RdsHook):
+def _create_db_instance(hook: RdsHook):
hook.conn.create_db_instance(
DBInstanceIdentifier=DB_INSTANCE_NAME,
- DBInstanceClass='db.m4.large',
- Engine='postgres',
+ DBInstanceClass="db.t4g.micro",
Review Comment:
Yes I ran the system tests. I don't remember it taking any longer than 20 minutes.
Try changing the *Availability and durability* in your console to "single instance" and you should be able to select db.t4g.micro, which is the least expensive instance type.
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r963169782
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -61,7 +66,14 @@ def _check_item(self, item_type: str, item_name: str) -> bool:
except ClientError:
return False
else:
- return bool(items) and any(map(lambda s: items[0]['Status'].lower() == s, self.target_statuses))
+ status_field = self._check_status_field()
+ return bool(items) and any(
+ map(lambda status: items[0][status_field].lower() == status, self.target_statuses)
+ )
Review Comment:
Thanks for the suggestions. I'd like to make these updates in a separate PR.
The formatting is the result of [black](https://black.readthedocs.io/en/stable/) (line too long)
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r962438077
##########
tests/providers/amazon/aws/sensors/test_rds.py:
##########
@@ -225,3 +233,67 @@ def test_export_task_poke_false(self):
dag=self.dag,
)
assert not op.poke(None)
+
+
+@pytest.mark.skipif(mock_rds is None, reason="mock_rds package not present")
+class TestRdsDbSensor:
+ @classmethod
+ def setup_class(cls):
+ cls.dag = DAG("test_dag", default_args={"owner": "airflow", "start_date": DEFAULT_DATE})
+ cls.hook = RdsHook(aws_conn_id=AWS_CONN, region_name="us-east-1")
+
+ @classmethod
+ def teardown_class(cls):
+ del cls.dag
+ del cls.hook
+
+ @mock_rds
+ def test_poke_true_instance(self):
+ """
+ By default RdsDbSensor should wait for an instance to enter the 'available' state
+ """
+ _create_db_instance(self.hook)
+ op = RdsDbSensor(
+ task_id="instance_poke_true",
+ db_identifier=DB_INSTANCE_NAME,
+ aws_conn_id=AWS_CONN,
+ dag=self.dag,
+ )
+ assert op.poke(None)
+
+ @mock_rds
+ def test_poke_false_instance(self):
+ _create_db_instance(self.hook)
+ op = RdsDbSensor(
+ task_id="instance_poke_false",
+ db_identifier=DB_INSTANCE_NAME,
+ target_statuses=["stopped"],
+ aws_conn_id=AWS_CONN,
+ dag=self.dag,
+ )
+ assert not op.poke(None)
+
+ @mock_rds
+ def test_poke_true_cluster(self):
+ _create_db_cluster(self.hook)
+ op = RdsDbSensor(
Review Comment:
2022/09/05 update
Added unit test cases for "cluster" db type
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r963196297
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -51,17 +50,29 @@ def _describe_item(self, item_type: str, item_name: str) -> list:
elif item_type == 'export_task':
exports = self.hook.conn.describe_export_tasks(ExportTaskIdentifier=item_name)
return exports['ExportTasks']
+ elif item_type == "db_instance":
+ instances = self.hook.conn.describe_db_instances(DBInstanceIdentifier=item_name)
+ return instances["DBInstances"]
+ elif item_type == "db_cluster":
+ clusters = self.hook.conn.describe_db_clusters(DBClusterIdentifier=item_name)
+ return clusters["DBClusters"]
else:
raise AirflowException(f"Method for {item_type} is not implemented")
def _check_item(self, item_type: str, item_name: str) -> bool:
"""Get certain item from `_describe_item()` and check its status"""
+ if item_type == "db_instance":
Review Comment:
Update 2022/09/06
https://github.com/apache/airflow/pull/26003#discussion_r963185967
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsInstanceSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r957892008
##########
tests/providers/amazon/aws/sensors/test_rds.py:
##########
@@ -50,15 +51,18 @@
EXPORT_TASK_SOURCE = 'arn:aws:rds:es-east-1::snapshot:my-db-instance-snap'
-def _create_db_instance_snapshot(hook: RdsHook):
+def _create_db_instance(hook: RdsHook):
hook.conn.create_db_instance(
DBInstanceIdentifier=DB_INSTANCE_NAME,
- DBInstanceClass='db.m4.large',
- Engine='postgres',
+ DBInstanceClass="db.t4g.micro",
Review Comment:
Yes I ran the system tests. I don't remember it taking any longer than 20 minutes.
Try changing the *Availability and durability* in your console to "single instance" and you should be able to select db.t4g.micro, which is the newest/least-expensive t-generation instance.
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsInstanceSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r956548215
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -38,10 +38,10 @@ def __init__(self, *args, aws_conn_id: str = "aws_conn_id", hook_params: Optiona
hook_params = hook_params or {}
self.hook = RdsHook(aws_conn_id=aws_conn_id, **hook_params)
self.target_statuses: List[str] = []
+ self.check_status_field = "Status"
Review Comment:
Current sensors rely on the same `botocore` response structure. The response to [`describe_db_instances`](https://botocore.amazonaws.com/v1/documentation/api/latest/reference/services/rds.html#RDS.Client.describe_db_instances) uses a different field name to describe the state of the requested resource. I chose to define the field name as an instance variable because..
- no need to update other sensor classes
- no need to modify method signatures
- minimal code changes
- it makes sense to define "sensor specific metadata" on the sensor class/instance
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsInstanceSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r956548866
##########
docs/apache-airflow-providers-amazon/operators/rds.rst:
##########
@@ -169,6 +169,22 @@ To delete a AWS DB instance you can use
Sensors
-------
+.. _howto/sensor:RdsInstanceSensor:
+
+Wait on an Amazon RDS instance status
+=====================================
+
+To wait for an Amazon RDS instance with specific statuses you can use
+:class:`~airflow.providers.amazon.aws.sensors.rds.RdsInstanceSensor`.
+By default, the sensor waits for the instance to reach the ``available`` state.
+
+.. exampleinclude:: /../../tests/system/providers/amazon/aws/rds/example_rds_instance.py
+ :language: python
+ :dedent: 4
+ :start-after: [START howto_sensor_rds_instance]
+ :end-before: [END howto_sensor_rds_instance]
+
Review Comment:
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r962436706
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -61,7 +66,14 @@ def _check_item(self, item_type: str, item_name: str) -> bool:
except ClientError:
return False
else:
- return bool(items) and any(map(lambda s: items[0]['Status'].lower() == s, self.target_statuses))
+ status_field = self._check_status_field()
Review Comment:
2022/09/05 update (https://github.com/apache/airflow/pull/26003/commits/0d537b1bdf1fa3dbd8a94f1b635a5814e84e11e9)
Replaced instance variable with instance method to encapsulate if/else statement
--
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
[GitHub] [airflow] hankehly commented on pull request #26003: Add RdsInstanceSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on PR #26003:
URL: https://github.com/apache/airflow/pull/26003#issuecomment-1229139725
@o-nikolas @vincbeck @ferruzzi @potiuk
Please review this at your earliest convenience.
--
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
[GitHub] [airflow] potiuk merged pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #26003:
URL: https://github.com/apache/airflow/pull/26003
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r962438077
##########
tests/providers/amazon/aws/sensors/test_rds.py:
##########
@@ -225,3 +233,67 @@ def test_export_task_poke_false(self):
dag=self.dag,
)
assert not op.poke(None)
+
+
+@pytest.mark.skipif(mock_rds is None, reason="mock_rds package not present")
+class TestRdsDbSensor:
+ @classmethod
+ def setup_class(cls):
+ cls.dag = DAG("test_dag", default_args={"owner": "airflow", "start_date": DEFAULT_DATE})
+ cls.hook = RdsHook(aws_conn_id=AWS_CONN, region_name="us-east-1")
+
+ @classmethod
+ def teardown_class(cls):
+ del cls.dag
+ del cls.hook
+
+ @mock_rds
+ def test_poke_true_instance(self):
+ """
+ By default RdsDbSensor should wait for an instance to enter the 'available' state
+ """
+ _create_db_instance(self.hook)
+ op = RdsDbSensor(
+ task_id="instance_poke_true",
+ db_identifier=DB_INSTANCE_NAME,
+ aws_conn_id=AWS_CONN,
+ dag=self.dag,
+ )
+ assert op.poke(None)
+
+ @mock_rds
+ def test_poke_false_instance(self):
+ _create_db_instance(self.hook)
+ op = RdsDbSensor(
+ task_id="instance_poke_false",
+ db_identifier=DB_INSTANCE_NAME,
+ target_statuses=["stopped"],
+ aws_conn_id=AWS_CONN,
+ dag=self.dag,
+ )
+ assert not op.poke(None)
+
+ @mock_rds
+ def test_poke_true_cluster(self):
+ _create_db_cluster(self.hook)
+ op = RdsDbSensor(
Review Comment:
2022/09/05 update (https://github.com/apache/airflow/pull/26003/commits/0d537b1bdf1fa3dbd8a94f1b635a5814e84e11e9)
Added unit test cases for "cluster" db type
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r962437210
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -149,7 +161,54 @@ def poke(self, context: 'Context'):
return self._check_item(item_type='export_task', item_name=self.export_task_identifier)
+class RdsDbSensor(RdsBaseSensor):
Review Comment:
2022/09/05 update
Renamed `RdsInstanceSensor` to `RdsDbSensor` because it considers "cluster" databases as well
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -149,7 +161,54 @@ def poke(self, context: 'Context'):
return self._check_item(item_type='export_task', item_name=self.export_task_identifier)
+class RdsDbSensor(RdsBaseSensor):
+ """
+ Waits for an RDS instance or cluster to enter one of a number of states
+
+ .. seealso::
+ For more information on how to use this sensor, take a look at the guide:
+ :ref:`howto/sensor:RdsDbSensor`
+
+ :param db_type: Type of the DB - either "instance" or "cluster"
+ :param db_identifier: The AWS identifier for the DB
+ :param target_statuses: Target status of DB
+ """
+
+ def __init__(
+ self,
+ *,
+ db_identifier: str,
+ db_type: str = RdsDbType.INSTANCE,
Review Comment:
2022/09/05 update
Added `db_type`
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -149,7 +161,53 @@ def poke(self, context: 'Context'):
return self._check_item(item_type='export_task', item_name=self.export_task_identifier)
+class RdsDbSensor(RdsBaseSensor):
+ """
+ Waits for an RDS instance or cluster to enter one of a number of states
+
+ .. seealso::
+ For more information on how to use this sensor, take a look at the guide:
+ :ref:`howto/sensor:RdsDbSensor`
+
+ :param db_type: Type of the DB - either "instance" or "cluster"
+ :param db_identifier: The AWS identifier for the DB
+ :param target_statuses: Target status of DB
+ """
+
+ def __init__(
+ self,
+ *,
+ db_identifier: str,
+ db_type: str = RdsDbType.INSTANCE,
+ target_statuses: Optional[List[str]] = None,
+ aws_conn_id: str = "aws_default",
+ **kwargs,
+ ):
+ super().__init__(aws_conn_id=aws_conn_id, **kwargs)
+ self.db_identifier = db_identifier
+ self.target_statuses = target_statuses or ["available"]
+ self.db_type = RdsDbType(db_type)
+
+ def poke(self, context: 'Context'):
+ self.log.info(
+ "Poking for statuses : %s\nfor db instance %s", self.target_statuses, self.db_identifier
+ )
+ item_type = self._check_item_type()
+ return self._check_item(item_type=item_type, item_name=self.db_identifier)
+
+ def _check_status_field(self) -> str:
+ if self.db_type == RdsDbType.INSTANCE:
Review Comment:
2022/09/05 update
Added `_check_status_field` and `_check_item_type` instance methods to account for different db types.
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r962462372
##########
docs/apache-airflow-providers-amazon/operators/rds.rst:
##########
@@ -169,6 +169,22 @@ To delete a AWS DB instance you can use
Sensors
-------
+.. _howto/sensor:RdsInstanceSensor:
+
+Wait on an Amazon RDS instance status
+=====================================
+
+To wait for an Amazon RDS instance with specific statuses you can use
+:class:`~airflow.providers.amazon.aws.sensors.rds.RdsInstanceSensor`.
+By default, the sensor waits for the instance to reach the ``available`` state.
+
+.. exampleinclude:: /../../tests/system/providers/amazon/aws/rds/example_rds_instance.py
+ :language: python
+ :dedent: 4
+ :start-after: [START howto_sensor_rds_instance]
+ :end-before: [END howto_sensor_rds_instance]
+
Review Comment:
2022/09/05 update (https://github.com/apache/airflow/pull/26003/commits/0d537b1bdf1fa3dbd8a94f1b635a5814e84e11e9)
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsInstanceSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r956549016
##########
tests/system/providers/amazon/aws/rds/example_rds_instance.py:
##########
@@ -46,7 +47,7 @@
create_db_instance = RdsCreateDbInstanceOperator(
task_id='create_db_instance',
db_instance_identifier=RDS_DB_IDENTIFIER,
- db_instance_class="db.m5.large",
+ db_instance_class="db.t4g.micro",
Review Comment:
In the off chance the system test fails to clean itself up..
--
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
[GitHub] [airflow] hankehly commented on pull request #26003: Add RdsInstanceSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on PR #26003:
URL: https://github.com/apache/airflow/pull/26003#issuecomment-1234921120
@o-nikolas @vincbeck @ferruzzi @potiuk
Could someone please merge this PR, or let me know what else needs to change
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsInstanceSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r962389259
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -38,10 +38,10 @@ def __init__(self, *args, aws_conn_id: str = "aws_conn_id", hook_params: Optiona
hook_params = hook_params or {}
self.hook = RdsHook(aws_conn_id=aws_conn_id, **hook_params)
self.target_statuses: List[str] = []
+ self.check_status_field = "Status"
Review Comment:
You're right, but that requires adding an `if-else` conditional statement. Is the current approach OK with you?
--
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
[GitHub] [airflow] kazanzhy commented on a diff in pull request #26003: Add RdsInstanceSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
kazanzhy commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r962372270
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -38,10 +38,10 @@ def __init__(self, *args, aws_conn_id: str = "aws_conn_id", hook_params: Optiona
hook_params = hook_params or {}
self.hook = RdsHook(aws_conn_id=aws_conn_id, **hook_params)
self.target_statuses: List[str] = []
+ self.check_status_field = "Status"
Review Comment:
I think this variable could be in `_check_item` method.
Based on `item_type` you could identify the status field
--
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
[GitHub] [airflow] kazanzhy commented on a diff in pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
kazanzhy commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r963135500
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -38,10 +38,10 @@ def __init__(self, *args, aws_conn_id: str = "aws_conn_id", hook_params: Optiona
hook_params = hook_params or {}
self.hook = RdsHook(aws_conn_id=aws_conn_id, **hook_params)
self.target_statuses: List[str] = []
+ self.check_status_field = "Status"
Review Comment:
I think it is OK to add one more `if-else`.
Actually, I meant something like:
```
def _check_item(self, item_type: str, item_name: str) -> bool:
"""Get certain item from `_describe_item()` and check its status"""
if item_type == 'db_instance':
status_field = 'DBInstanceStatus'
else:
status_field = 'Status'
try:
items = self._describe_item(item_type, item_name)
except ClientError:
return False
else:
return bool(items) and any(
map(lambda status: items[0][status_field].lower() == status, self.target_statuses)
)
```
WDYT?
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsInstanceSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r956548480
##########
docs/apache-airflow-providers-amazon/operators/rds.rst:
##########
@@ -169,6 +169,22 @@ To delete a AWS DB instance you can use
Sensors
-------
+.. _howto/sensor:RdsInstanceSensor:
+
+Wait on an Amazon RDS instance status
+=====================================
+
+To wait for an Amazon RDS instance with specific statuses you can use
+:class:`~airflow.providers.amazon.aws.sensors.rds.RdsInstanceSensor`.
+By default, the sensor waits for the existence of a snapshot with status ``available``.
+
+.. exampleinclude:: /../../tests/system/providers/amazon/aws/rds/example_rds_instance.py
+ :language: python
+ :dedent: 4
+ :start-after: [START howto_sensor_rds_instance]
+ :end-before: [END howto_sensor_rds_instance]
+
Review Comment:
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r963185967
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -38,10 +38,10 @@ def __init__(self, *args, aws_conn_id: str = "aws_conn_id", hook_params: Optiona
hook_params = hook_params or {}
self.hook = RdsHook(aws_conn_id=aws_conn_id, **hook_params)
self.target_statuses: List[str] = []
+ self.check_status_field = "Status"
Review Comment:
By adding if/else logic to the base class, we're making it "aware" of the details of the subclass. I think a better approach is to let the subclass tell us what it needs.
I checked the `botocore` documentation and it looks like "DBInstanceStatus" and "Status" are the only 2 options, so I'll go ahead and make the change to finish up this PR.
--
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
[GitHub] [airflow] kazanzhy commented on a diff in pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
kazanzhy commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r963138280
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -61,7 +66,14 @@ def _check_item(self, item_type: str, item_name: str) -> bool:
except ClientError:
return False
else:
- return bool(items) and any(map(lambda s: items[0]['Status'].lower() == s, self.target_statuses))
+ status_field = self._check_status_field()
+ return bool(items) and any(
+ map(lambda status: items[0][status_field].lower() == status, self.target_statuses)
+ )
Review Comment:
Also, the `return` could be modified from `map` to the generator if you wish.
```suggestion
return bool(items) and any(items[0][status_field].lower() == status for status in self.target_statuses)
```
For me, both variants are well readable but maybe this one is better. WDYT?
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r962435821
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -51,6 +50,12 @@ def _describe_item(self, item_type: str, item_name: str) -> list:
elif item_type == 'export_task':
exports = self.hook.conn.describe_export_tasks(ExportTaskIdentifier=item_name)
return exports['ExportTasks']
+ elif item_type == "db_instance":
+ instances = self.hook.conn.describe_db_instances(DBInstanceIdentifier=item_name)
+ return instances["DBInstances"]
+ elif item_type == "db_cluster":
Review Comment:
Added `db_cluster` in https://github.com/apache/airflow/pull/26003/commits/0d537b1bdf1fa3dbd8a94f1b635a5814e84e11e9
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsInstanceSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r956548215
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -38,10 +38,10 @@ def __init__(self, *args, aws_conn_id: str = "aws_conn_id", hook_params: Optiona
hook_params = hook_params or {}
self.hook = RdsHook(aws_conn_id=aws_conn_id, **hook_params)
self.target_statuses: List[str] = []
+ self.check_status_field = "Status"
Review Comment:
Current sensors rely on the same `botocore` response structure. The response to [`describe_db_instances`](https://botocore.amazonaws.com/v1/documentation/api/latest/reference/services/rds.html#RDS.Client.describe_db_instances) uses a different field name to describe the state of the requested resource. I chose to define the field name as an instance variable because..
- no need to update other sensor classes
- no need to modify method signatures
- minimal code changes
- no extra conditional statements
- it makes sense to define "sensor specific metadata" on the sensor class/instance
--
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
[GitHub] [airflow] ferruzzi commented on a diff in pull request #26003: Add RdsInstanceSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
ferruzzi commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r957523872
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -61,7 +64,9 @@ def _check_item(self, item_type: str, item_name: str) -> bool:
except ClientError:
return False
else:
- return bool(items) and any(map(lambda s: items[0]['Status'].lower() == s, self.target_statuses))
+ return bool(items) and any(
+ map(lambda s: items[0][self.check_status_field].lower() == s, self.target_statuses)
Review Comment:
Nitpick: Can you rename `s` here to something useful?
##########
tests/providers/amazon/aws/sensors/test_rds.py:
##########
@@ -50,15 +51,18 @@
EXPORT_TASK_SOURCE = 'arn:aws:rds:es-east-1::snapshot:my-db-instance-snap'
-def _create_db_instance_snapshot(hook: RdsHook):
+def _create_db_instance(hook: RdsHook):
hook.conn.create_db_instance(
DBInstanceIdentifier=DB_INSTANCE_NAME,
- DBInstanceClass='db.m4.large',
- Engine='postgres',
+ DBInstanceClass="db.t4g.micro",
Review Comment:
Have you tested this? The 'large' is the smallest option on the AWS console when creating a new RDS instance. In fact, I just checked to verify that and the m4 generation isn't even an option in their dropdown anymore, it's only gen 5 or gen 6 now. I just want to make sure it'll run alright with the lower specs. I know the RDS Export system test takes about 20 minutes to run on the `large`, I'm not sure how much longer this is going to add.
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsInstanceSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r956548910
##########
tests/providers/amazon/aws/sensors/test_rds.py:
##########
@@ -50,15 +51,18 @@
EXPORT_TASK_SOURCE = 'arn:aws:rds:es-east-1::snapshot:my-db-instance-snap'
-def _create_db_instance_snapshot(hook: RdsHook):
+def _create_db_instance(hook: RdsHook):
hook.conn.create_db_instance(
DBInstanceIdentifier=DB_INSTANCE_NAME,
- DBInstanceClass='db.m4.large',
- Engine='postgres',
+ DBInstanceClass="db.t4g.micro",
Review Comment:
I took the liberty of changing the db instance class to the smallest possible size here and in the system test.
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsInstanceSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r957898949
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -61,7 +64,9 @@ def _check_item(self, item_type: str, item_name: str) -> bool:
except ClientError:
return False
else:
- return bool(items) and any(map(lambda s: items[0]['Status'].lower() == s, self.target_statuses))
+ return bool(items) and any(
+ map(lambda s: items[0][self.check_status_field].lower() == s, self.target_statuses)
Review Comment:
Thanks, updated in https://github.com/apache/airflow/pull/26003/commits/82813173d05782562dfae3b68394df3c8082d03a
--
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
[GitHub] [airflow] kazanzhy commented on a diff in pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
kazanzhy commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r963135500
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -38,10 +38,10 @@ def __init__(self, *args, aws_conn_id: str = "aws_conn_id", hook_params: Optiona
hook_params = hook_params or {}
self.hook = RdsHook(aws_conn_id=aws_conn_id, **hook_params)
self.target_statuses: List[str] = []
+ self.check_status_field = "Status"
Review Comment:
I think it is OK to add one more `if-else`.
Actually, I meant something like:
```
def _check_item(self, item_type: str, item_name: str) -> bool:
"""Get certain item from `_describe_item()` and check its status"""
if item_type == 'db_instance':
status_field = 'DBInstanceStatus'
else:
status_field = 'Status'
try:
items = self._describe_item(item_type, item_name)
except ClientError:
return False
else:
return bool(items) and any(
map(lambda status: items[0][status_field].lower() == status, self.target_statuses)
)
```
WDYT?
--
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
[GitHub] [airflow] kazanzhy commented on a diff in pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
kazanzhy commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r963135500
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -38,10 +38,10 @@ def __init__(self, *args, aws_conn_id: str = "aws_conn_id", hook_params: Optiona
hook_params = hook_params or {}
self.hook = RdsHook(aws_conn_id=aws_conn_id, **hook_params)
self.target_statuses: List[str] = []
+ self.check_status_field = "Status"
Review Comment:
I think it is OK to add one more `if-else`.
Actually, I meant something like:
```
def _check_item(self, item_type: str, item_name: str) -> bool:
"""Get certain item from `_describe_item()` and check its status"""
if item_type == 'db_instance':
status_field = 'DBInstanceStatus'
else:
status_field = 'Status'
try:
items = self._describe_item(item_type, item_name)
except ClientError:
return False
else:
return bool(items) and any(
map(lambda status: items[0][status_field].lower() == status, self.target_statuses)
)
```
Seems only `DBInstance` has this issue with the Status field.
WDYT?
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r963187679
##########
airflow/providers/amazon/aws/sensors/rds.py:
##########
@@ -38,10 +38,10 @@ def __init__(self, *args, aws_conn_id: str = "aws_conn_id", hook_params: Optiona
hook_params = hook_params or {}
self.hook = RdsHook(aws_conn_id=aws_conn_id, **hook_params)
self.target_statuses: List[str] = []
+ self.check_status_field = "Status"
Review Comment:
Changed in https://github.com/apache/airflow/pull/26003/commits/afefe365706c625bc0deebbf9bc51362ce2bf367
--
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
[GitHub] [airflow] hankehly commented on a diff in pull request #26003: Add RdsDbSensor to amazon provider package
Posted by GitBox <gi...@apache.org>.
hankehly commented on code in PR #26003:
URL: https://github.com/apache/airflow/pull/26003#discussion_r962462372
##########
docs/apache-airflow-providers-amazon/operators/rds.rst:
##########
@@ -169,6 +169,22 @@ To delete a AWS DB instance you can use
Sensors
-------
+.. _howto/sensor:RdsInstanceSensor:
+
+Wait on an Amazon RDS instance status
+=====================================
+
+To wait for an Amazon RDS instance with specific statuses you can use
+:class:`~airflow.providers.amazon.aws.sensors.rds.RdsInstanceSensor`.
+By default, the sensor waits for the instance to reach the ``available`` state.
+
+.. exampleinclude:: /../../tests/system/providers/amazon/aws/rds/example_rds_instance.py
+ :language: python
+ :dedent: 4
+ :start-after: [START howto_sensor_rds_instance]
+ :end-before: [END howto_sensor_rds_instance]
+
Review Comment:
2022/09/05 update
--
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