-
Notifications
You must be signed in to change notification settings - Fork 14.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
adds support for querying Redshift Serverless clusters #32785
adds support for querying Redshift Serverless clusters #32785
Conversation
ed74f82
to
7cc9dae
Compare
7cc9dae
to
56cc6d8
Compare
) -> str: | ||
""" | ||
Execute a statement against Amazon Redshift. | ||
|
||
:param database: the name of the database | ||
:param sql: the SQL statement or list of SQL statement to run | ||
:param cluster_identifier: unique identifier of a cluster | ||
:param workgroup_name: name of the Redshift Serverless workgroup. Mutually exclusive with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: i think we should reorder the docstring as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Absolutely :) addressed in 71bea87
@@ -40,6 +40,9 @@ class RedshiftDataOperator(BaseOperator): | |||
:param database: the name of the database | |||
:param sql: the SQL statement or list of SQL statement to run | |||
:param cluster_identifier: unique identifier of a cluster | |||
:param workgroup_name: name of the Redshift Serverless workgroup. Mutually exclusive with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: same here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here: addressed in 71bea87 :)
@@ -55,6 +58,7 @@ class RedshiftDataOperator(BaseOperator): | |||
|
|||
template_fields = ( | |||
"cluster_identifier", | |||
"workgroup_name", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure whether this will need to be reordered
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also not sure, but for consistency addressed in 71bea87
Besides the nits mentioned by @Lee-W , it looks good to me :) Good job! |
56cc6d8
to
3a1189e
Compare
serverless clusters require the `workgroup_name` to be specified and do not require the `cluster_identifier`
3a1189e
to
71bea87
Compare
Adds support for querying Redshift Serverless clusters using the
RedshiftDataOperator
. Serverless clusters require theworkgroup_name
to be specified and do not require thecluster_identifier
.Closes #32280
Tested with this DAG:
and with using my personal AWS credentials, the result showed that the current user is
IAM:ikolenkas
, which is correct.^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rst
or{issue_number}.significant.rst
, in newsfragments.