Removed unnecessary aws_conn_id param from operators constructors#51236
Removed unnecessary aws_conn_id param from operators constructors#51236vincbeck merged 3 commits intoapache:mainfrom
Conversation
boring-cyborg
bot
added
area:providers
provider:amazon
vincbeck
left a comment
vincbeck
left a comment
There was a problem hiding this comment.
LGTM! Thanks for the cleanup. Related to the discussion here, I think we should not keep parameter from parent classes in docstring. Let's agree first on whether we want to keep them, and then, potentially, you can update this PR
|
Thanks for opening this! I'm still in favour of keeping the doc strings (I replied on the other thread I started that Vincent linked). |
o-nikolas
left a comment
o-nikolas
left a comment
There was a problem hiding this comment.
Can you add tests for these operators like you did for the RDS case? I know not all of them have it, but it's a great mechanism to ensure we don't regress, so if you're modifying these ops anyway, adding the same tests would be great.
I added them for all the operators I have adjusted before + I have renamed the test: no_conn to default_conn to avoid any misunderstandings in the future. If u want me to add them to other operators etc. lmk! |
3 participants
As discussed on #51196 there are some more operators, that set the
aws_conn_idin their constructor, even though it is set by the base operator from which they inherit. This does not lead to any errors, but is not consistent, which is why I removed it. Feel free to close the PR if you think it's unnecessary.