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 2021/12/17 10:01:34 UTC
[GitHub] [airflow] ferruzzi opened a new pull request #20374: Standardize AWS Redshift naming
ferruzzi opened a new pull request #20374:
URL: https://github.com/apache/airflow/pull/20374
Part of [#20296](https://github.com/apache/airflow/issues/20296)
Also changed `poke(None)` to `poke({})` because IDE type hinting warnings (expected dict, received NoneType)
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
--
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] eladkal merged pull request #20374: Standardize AWS Redshift naming
Posted by GitBox <gi...@apache.org>.
eladkal merged pull request #20374:
URL: https://github.com/apache/airflow/pull/20374
--
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] github-actions[bot] commented on pull request #20374: Standardize AWS Redshift naming
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #20374:
URL: https://github.com/apache/airflow/pull/20374#issuecomment-1006315438
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
--
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 edited a comment on pull request #20374: Standardize AWS Redshift naming
Posted by GitBox <gi...@apache.org>.
ferruzzi edited a comment on pull request #20374:
URL: https://github.com/apache/airflow/pull/20374#issuecomment-1005258276
rebased and I think I covered https://github.com/apache/airflow/pull/20374#discussion_r771918489
--
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] eladkal commented on pull request #20374: Standardize AWS Redshift naming
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #20374:
URL: https://github.com/apache/airflow/pull/20374#issuecomment-1006729517
Merging for now as it's pretty much the same to all other strandrization PRs
If there will be additional comments we'll address them in a follow up 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] ferruzzi commented on pull request #20374: Standardize AWS Redshift naming
Posted by GitBox <gi...@apache.org>.
ferruzzi commented on pull request #20374:
URL: https://github.com/apache/airflow/pull/20374#issuecomment-1005268141
This one is throwing the Protocol inheritance error that I mentioned in the other PR: https://github.com/apache/airflow/runs/4708697673?check_suite_focus=true#step:8:8489
--
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] eladkal commented on a change in pull request #20374: Standardize AWS Redshift naming
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #20374:
URL: https://github.com/apache/airflow/pull/20374#discussion_r771535786
##########
File path: airflow/providers/amazon/aws/sensors/redshift_cluster.py
##########
@@ -58,3 +58,19 @@ def get_hook(self) -> RedshiftHook:
self.hook = RedshiftHook(aws_conn_id=self.aws_conn_id)
return self.hook
+
+
+class AwsRedshiftClusterSensor(RedshiftClusterSensor):
Review comment:
We dont need deprecation here.
This is a new class added recently in https://github.com/apache/airflow/pull/20276/files
This was never released to users so we can avoid this deprecation.
There is no point in releasing a class which is already deprecated :)
We should redirect from
https://github.com/apache/airflow/blob/main/airflow/providers/amazon/aws/sensors/redshift.py
directly to the `RedshiftClusterSensor`
--
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] eladkal commented on a change in pull request #20374: Standardize AWS Redshift naming
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #20374:
URL: https://github.com/apache/airflow/pull/20374#discussion_r771918489
##########
File path: airflow/providers/amazon/aws/sensors/redshift.py
##########
@@ -17,12 +17,12 @@
# under the License.
import warnings
Review comment:
Since `airflow.providers.amazon.aws.sensors.redshift` is a file that already released (long ago actually) We can't do here what I suggested for `redshift_cluster` in https://github.com/apache/airflow/pull/20374#discussion_r771535786
Here we need to make sure that both `AwsRedshiftClusterSensor` and `RedshiftClusterSensor` are working to the users.
--
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] github-actions[bot] commented on pull request #20374: Standardize AWS Redshift naming
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #20374:
URL: https://github.com/apache/airflow/pull/20374#issuecomment-996610437
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.
--
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] eladkal commented on a change in pull request #20374: Standardize AWS Redshift naming
Posted by GitBox <gi...@apache.org>.
eladkal commented on a change in pull request #20374:
URL: https://github.com/apache/airflow/pull/20374#discussion_r771535786
##########
File path: airflow/providers/amazon/aws/sensors/redshift_cluster.py
##########
@@ -58,3 +58,19 @@ def get_hook(self) -> RedshiftHook:
self.hook = RedshiftHook(aws_conn_id=self.aws_conn_id)
return self.hook
+
+
+class AwsRedshiftClusterSensor(RedshiftClusterSensor):
Review comment:
We dont need deprecation here.
This is a new class added recently in https://github.com/apache/airflow/pull/20276/files
This was never released to users so we can avoid this deprecation.
We should redirect from
https://github.com/apache/airflow/blob/main/airflow/providers/amazon/aws/sensors/redshift.py
directly to the `RedshiftClusterSensor`
--
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 #20374: Standardize AWS Redshift naming
Posted by GitBox <gi...@apache.org>.
ferruzzi commented on pull request #20374:
URL: https://github.com/apache/airflow/pull/20374#issuecomment-1005258276
rebased and I think I covered https://github.com/apache/airflow/pull/20374#discussion_r771535786
--
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