[
https://issues.apache.org/jira/browse/AIRFLOW-5720?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Kamil Bregula resolved AIRFLOW-5720.
------------------------------------
Resolution: Won't Fix
Resolved in different way:
https://github.com/apache/airflow/pull/6390#issuecomment-554045581
> don't call _get_connections_from_db in TestCloudSqlDatabaseHook
> ---------------------------------------------------------------
>
> Key: AIRFLOW-5720
> URL: https://issues.apache.org/jira/browse/AIRFLOW-5720
> Project: Apache Airflow
> Issue Type: New Feature
> Components: gcp
> Affects Versions: 1.10.5
> Reporter: Daniel Standish
> Assignee: Daniel Standish
> Priority: Major
>
> Issues with this test class:
> *tests are mocking lower-level than they need to*
> Tests were mocking {{airflow.hook.BaseHook.get_connections}}.
> Instead they can mock
> {{airflow.gcp.hooks.cloud_sql.CloudSqlDatabaseHook.get_connection}} which is
> more direct.
> *should not reference private method*
> This is an impediment to refactoring of connections / creds.
> *Tests had complexity that did not add a benefit*
> They all had this bit:
> {code:python}
> self._setup_connections(get_connections, uri)
> gcp_conn_id = 'google_cloud_default'
> hook = CloudSqlDatabaseHook(
>
> default_gcp_project_id=BaseHook.get_connection(gcp_conn_id).extra_dejson.get(
> 'extra__google_cloud_platform__project')
> )
> {code}
> {{_setup_connections}} was like this:
> {code:python}
> @staticmethod
> def _setup_connections(get_connections, uri):
> gcp_connection = mock.MagicMock()
> gcp_connection.extra_dejson = mock.MagicMock()
> gcp_connection.extra_dejson.get.return_value = 'empty_project'
> cloudsql_connection = Connection()
> cloudsql_connection.parse_from_uri(uri)
> cloudsql_connection2 = Connection()
> cloudsql_connection2.parse_from_uri(uri)
> get_connections.side_effect = [[gcp_connection],
> [cloudsql_connection],
> [cloudsql_connection2]]
> {code}
> Issues here are as follows.
> 1. no test ever used the third side effect
> 2. the first side effect does not help us; {{default_gcp_project_id}} is
> irrelevant
> Only one of the three connections in {{_setup_connections}} has any impact on
> the test.
> The call of {{BaseHook.get_connection}} only serves to discard the first
> connection in mock side effect list, {{gcp_connection}}.
> The second connection is the one that matters, and it is returned when
> {{CloudSqlDatabaseHook}} calls `self.get_connection` during init. Since it
> is a mock side effect, it doesn't matter what value is given for conn_id. So
> the {{CloudSqlDatabaseHook}} init param {{default_gcp_project_id}} has no
> consequence. And because it has no consequence, we should not supply a value
> for it because this is misleading.
> We should not have extra code that does not serve a purpose because it makes
> it harder to understand what's actually happening.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
