Split redshift cluster and redshift sql

[dstandish]

The first redshift hook was for managing the cluster itself. Later a hook for using the cluster was added to the same module. Better to separate these into distinct modules redshift_sql and redshift_cluster. Here we split the redshift modules for operators, hooks, and sensors.

[github-actions]

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.

[potiuk]

The deprecation warnings should be added to "known" list and tests are failing unfortunately :)

[dstandish]

The deprecation warnings should be added to "known" list and tests are failing unfortunately :)

thanks wil work on it

[dstandish]

The deprecation warnings should be added to "known" list and tests are failing unfortunately :)

Hey @potiuk , can you point me the way here...

Providers manager is warning here:

                        if already_registered.hook_class_name != hook_class_name:
                            log.warning(
                                "The hook connection type '%s' is registered twice in the"
                                " package '%s' with different class names: '%s' and '%s'. "
                                " Please fix it!",
                                hook_info.connection_type,
                                package_name,
                                already_registered.hook_class_name,
                                hook_class_name,
                            )

This causes a test to fail. I did not change anything about the class names or connection types. They're now in different modules though.

Do you know what to do here? Perhaps we should not have let conn_type be redshift for the RedshiftSqlHook when it was added.

[potiuk]

Providers manager is warning here:
if already_registered.hook_class_name != hook_class_name:

This warning is not in the CI. I believe this is caused by .pyc file which remained from the Hook being present in the old location when you moved the .py file (in your local development environment). This is one of the main reasons in Breeze we have this:

    # Disable writing .pyc files - slightly slower imports but not messing around when switching
    # Python version and avoids problems with root-owned .pyc files in host
    export PYTHONDONTWRITEBYTECODE=${PYTHONDONTWRITEBYTECODE:="true"}

cc: @uranusjr (our slack discussion from yesterday) -> this is one of the bad sides of the .pyc files, that's why I always disable writing .pyc files in any development environment I have. Renaming and moving modules/packages causes this kind of problems.

[dstandish]

Providers manager is warning here:
if already_registered.hook_class_name != hook_class_name:

This warning is not in the CI. I believe this is caused by .pyc file which remained from the Hook being present in the old location when you moved the .py file (in your local development environment). This is one of the main reasons in Breeze we have this:

    # Disable writing .pyc files - slightly slower imports but not messing around when switching
    # Python version and avoids problems with root-owned .pyc files in host
    export PYTHONDONTWRITEBYTECODE=${PYTHONDONTWRITEBYTECODE:="true"}

cc: @uranusjr (our slack discussion from yesterday) -> this is one of the bad sides of the .pyc files, that's why I always disable writing .pyc files in any development environment I have. Renaming and moving modules/packages causes this kind of problems.

but it is in ci @potiuk 🤔

see: https://github.com/apache/airflow/runs/4515244414?check_suite_focus=true#step:6:12103

[potiuk]

Ah. i see.

See the FULL stack trace there (The summary of error does not have the warning just assert - but in the output of the test you have the whole error message:

https://github.com/apache/airflow/runs/4515244414?check_suite_focus=true#step:6:4063

[potiuk]

You need to unfold the failed "core" tests to see it all:

  
  tests/core/test_providers_manager.py:151: AssertionError
  ----------------------------- Captured stdout call -----------------------------
  [2021-12-14 03:43:05,390] {providers_manager.py:511} WARNING - The hook connection type 'aws' is registered twice in the package 'apache-airflow-providers-amazon' with different class names: 'airflow.providers.amazon.aws.hooks.base_aws.AwsBaseHook' and 'airflow.providers.amazon.aws.hooks.redshift_cluster.RedshiftHook'.  Please fix it!
  ------------------------------ Captured log call -------------------------------
  WARNING  airflow.providers_manager:providers_manager.py:511 The hook connection type 'aws' is registered twice in the package 'apache-airflow-providers-amazon' with different class names: 'airflow.providers.amazon.aws.hooks.base_aws.AwsBaseHook' and 'airflow.providers.amazon.aws.hooks.redshift_cluster.RedshiftHook'.  Please fix it!
  ________________________ TestProviderManager.test_hooks ________________________

Is it clearer :) ?

[eladkal]

@dstandish would it be possible to handle the class name changes on this PR as well?
#20296
The modifications for that are relatively small.

We shouldn't have AwsRedshiftClusterSensor. It should be:
from airflow.providers.amazon.aws.sensors.redshift_cluster import RedshiftClusterSensor

[uranusjr]

@dstandish There are more deprecation messages that need changing

[dstandish]

@dstandish There are more deprecation messages that need changing

Ok will search again tomorrow thought I got everything

made the changes

[eladkal]

LGTM

we will need a follow up on renaming AwsRedshiftClusterSensor #20296

[dstandish]

thanks everyone for the help in [hopefully] getting this one right