You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/09/22 16:39:56 UTC

[GitHub] [airflow] pauldalewilliams commented on a diff in pull request #26576: Add oracledb thick mode support for oracle provider

pauldalewilliams commented on code in PR #26576:
URL: https://github.com/apache/airflow/pull/26576#discussion_r977875742


##########
tests/providers/oracle/hooks/test_oracle.py:
##########
@@ -82,6 +102,7 @@ def test_get_conn_sid(self, mock_connect):
     def test_get_conn_service_name(self, mock_connect):
         dsn_service_name = {'dsn': 'ignored', 'service_name': 'service_name'}
         self.connection.extra = json.dumps(dsn_service_name)
+        self.db_hook.__init__()

Review Comment:
   I did because of the way the tests were originally structured.  They create the hook in `setUp` (line 64 in my commit) and then modify the extra in the tests themselves.  Since all the parameter parsing is now happening in the `__init__` it never picks that up.  This seemed to work for picking up those changes without drastically changing the tests.  I considered trying to rework it to function more like the way tests are structured in https://github.com/apache/airflow/blob/main/tests/providers/ssh/hooks/test_ssh.py with a separate connection for each variation.  But I'm not very confident in writing tests in the first place and didn't fully understand how to rewrite all this in that way.
   
   I did feel a little goofy doing it this way but it seemed to work for the purpose just fine and I couldn't think of any negatives aside from it just being goofy.



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