[GitHub] [airflow] uranusjr commented on a change in pull request #21699: Fix oracle test connection

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

uranusjr commented on a change in pull request #21699:
URL: https://github.com/apache/airflow/pull/21699#discussion_r810824368



##########
File path: airflow/providers/oracle/hooks/oracle.py
##########
@@ -339,3 +339,16 @@ def handler(cursor):
         )
 
         return result
+
+    def test_connection(self):
+        """Tests the connection by executing a select 1 from dual query"""
+        status, message = False, ''
+        try:
+            if self.get_first("select 1 from dual"):
+                status = True
+                message = 'Connection successfully tested'
+        except Exception as e:
+            status = False
+            message = str(e)
+
+        return status, message

Review comment:
       While this works (and is the best approach due to backward compatibility considerations), we should look into eventually merge this implementation back into `DbApiHook`. Can you refactor `DbApiHook` to something like this:
   
   ```python
   class DbApiHook:
       _test_connection_sql = "select 1"
   
       def test_connection(self):
           ...
           if self.get_first(self._test_connection_sql):"
               ...
   ```
   
   And add a comment here like this?
   
   ```python
   # TODO: Merge this implementation back to DbApiHook when dropping
   # support for Airflow 2.2.
   ```




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