Update Amazon RDS Operators and Sensors to inherit AWS Base classes #48872
Conversation
boring-cyborg
bot
added
area:providers
kind:documentation
provider:amazon
…pache#48872) * Update RDS operator to inherit AwsBaseOperator --------- Co-authored-by: mse139 <mse139@users.noereply.github.com>
| :param tags: A dictionary of tags or a list of tags in format `[{"Key": "...", "Value": "..."},]` | ||
| `USER Tagging <https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/USER_Tagging.html>`__ | ||
| :param wait_for_completion: If True, waits for creation of the DB snapshot to complete. (default: True) | ||
| :param aws_conn_id: The Airflow connection used for AWS credentials. |
There was a problem hiding this comment.
Any reason you included aws_conn_id param here but not for any of the other operators?
There was a problem hiding this comment.
No, looks like an oversight when adding aws_conn_id,region_name, and verify. I can do another MR to add it to the other operators.
There was a problem hiding this comment.
I am not a fan of copying parent class docstrings
There was a problem hiding this comment.
Same, I think we should not include docstrings from parent classes. If there are multiple parents, the list can become long and some docstring can quickly become out of date. Another example: #51236
There was a problem hiding this comment.
How about adding a:
seealso::
:class: <RespectiveBaseOperator> regarding additional parameters
Although this could lead to a “chaining” in case of 3+ level inheritation
There was a problem hiding this comment.
I think this is more about maintaining consistency with these 4 important params. We're trying to signal to the user (and their IDE via param driven autocompletion) what can still be done with these operators, despite those params being moved to the super class. So for me personally, I'm still in favour of this. But as usual, if I'm out voted we can stop doing it and go back and remove all the places where we have (there are a LOT so that will be a a great line count booster PR for someone 😉 )
5 participants
Partially addresses #35278 - RDS operators/sensors. Triggers already inherit AWS base classes.