[GitHub] [airflow] uranusjr commented on pull request #17515: Initializes RedshiftSqlHook by extending the PostgresHook

[GitBox <gi...@apache.org>]

uranusjr commented on pull request #17515:
URL: https://github.com/apache/airflow/pull/17515#issuecomment-908235537


   While I understand the superiority in the design aspect of putting everything in `RedshiftHook`, it is quite resource-consuming to implement since `AwsBaseHook` and `PostgresHook` have very different features and you can’t easily subclass them both. So the only choice would be composition, i.e. something like
   
   ```python
   class RedshiftHook(BaseHook):
       def __init__(self, ...):
           ...
           self._aws_hook = AwsBaseHook(...)
           self._sql_hook = PostgresHook(...)
   
   	# Cluster functions...
       def cluster_status(self, cluster_identifier: str) -> str:
           return self._aws_hook.get_conn()...
   
   	# SQL functions...
       def get_records(self, sql, parameters=None):
           return self._sql_hook.get_records(sql, parameters=parameters)
   ```
   
   It would be non-trivial to get everything line up correctly and cause maintenance burden. Splitting the AWS and SQL functionalities into two hooks is the better engineering solution IMO.


-- 
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