[GitHub] [airflow] kazanzhy commented on pull request #19639: Draft: Remove `allowed_conn_types` from SqlSensor

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

kazanzhy commented on pull request #19639:
URL: https://github.com/apache/airflow/pull/19639#issuecomment-983620791


   @potiuk @uranusjr @SamWheating 
   I did my best and seems the tests are passed.
   But I realize that this solution is not elegant and might be not correct, therefore I want to ask for advice from more experienced colleagues.
   
   In this PR there is a known problem of `mock` and `isinstance` conflict. I tried to patch different classes and got the following issues:
   ```
   'airflow.sensors.sql.DbApiHook' -> TypeError: isinstance() arg 2 must be a class, type, or tuple of classes and types
   'airflow.sensors.sql.BaseHook' -> The associated hook should be a subclass of `DbApiHook`. Got MagicMock
   'airflow.hooks.base.BaseHook' -> Does not work as mock and trying to connect to database
   ```
   
   Here is my toy script for testing this situation:
   ```
   import unittest
   from unittest import mock
   
   class Connection:
       def get_hook(self):
           return BigQeryHook()
   
   class BaseHook:
       @classmethod
       def get_connection(cls):
           return Connection()
   
   class DbApiHook(BaseHook): pass
   
   class BigQeryHook(DbApiHook): pass
   
   class SqlSensor:
       def _get_hook(self):
           hook = BaseHook.get_connection().get_hook()
           print('Sensor:', type(hook), hook)
           if not isinstance(hook, DbApiHook):
               raise Exception(f'The associated hook should be a subclass of `DbApiHook`. Got {hook.__class__.__name__}')
       def poke(self):
           self._get_hook()
   
   class TestSqlSensor1(unittest.TestCase):
       @mock.patch('__main__.BaseHook')
       def test_something(self, mock_hook):
           #mock_hook.get_connection.return_value.get_hook.return_value = mock.MagicMock(spec=DbApiHook)
           SqlSensor().poke()
   
   class TestSqlSensor2(unittest.TestCase):
       @mock.patch('__main__.DbApiHook')
       def test_something(self, mock_hook):
           SqlSensor().poke()
   
   if __name__ == '__main__':
       SqlSensor().poke()
       unittest.main()
   ```


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