[GitHub] [airflow] dstandish commented on a change in pull request #18431: Add hook_params in SqlSensor.

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

dstandish commented on a change in pull request #18431:
URL: https://github.com/apache/airflow/pull/18431#discussion_r750468458



##########
File path: airflow/sensors/sql.py
##########
@@ -58,21 +61,34 @@ class SqlSensor(BaseSensorOperator):
     ui_color = '#7c7287'
 
     def __init__(
-        self, *, conn_id, sql, parameters=None, success=None, failure=None, fail_on_empty=False, **kwargs
+        self, 
+        *, 
+        conn_id, 
+        sql, 
+        parameters=None, 
+        success=None, 
+        failure=None, 
+        fail_on_empty=False, 
+        hook_params={}, 

Review comment:
       same as above

##########
File path: airflow/models/connection.py
##########
@@ -289,8 +289,11 @@ def rotate_fernet_key(self):
         if self._extra and self.is_extra_encrypted:
             self._extra = fernet.rotate(self._extra.encode('utf-8')).decode()
 
-    def get_hook(self):
-        """Return hook based on conn_type."""
+    def get_hook(self, hook_params: Dict = {}):

Review comment:
       I understand your point @kazanzhy but I agree we should just stick with the practice of using `None` as the default for `Optional[Dict]` that is, as far as I know, uniformly adhered to in this repo.
   
   If you leave it, people will always give it a second look when they see it.  And pycharm complains about it.  And using `None` for an optional arg is also a common pattern that I think is good aside from the mutability issue, just sends the message, you don't need to pass this, and we don't forward anything in this case.
   




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