[GitHub] [airflow] potiuk commented on a diff in pull request #31925: Install sqlalchemy-spanner package into Google provider

["potiuk (via GitHub)" <gi...@apache.org>]

potiuk commented on code in PR #31925:
URL: https://github.com/apache/airflow/pull/31925#discussion_r1270491815


##########
airflow/providers/google/cloud/hooks/spanner.py:
##########
@@ -18,28 +18,43 @@
 """This module contains a Google Cloud Spanner Hook."""
 from __future__ import annotations
 
-from typing import Callable, Sequence
+from typing import Callable, NamedTuple, Sequence
 
 from google.api_core.exceptions import AlreadyExists, GoogleAPICallError
 from google.cloud.spanner_v1.client import Client
 from google.cloud.spanner_v1.database import Database
 from google.cloud.spanner_v1.instance import Instance
 from google.cloud.spanner_v1.transaction import Transaction
 from google.longrunning.operations_grpc_pb2 import Operation
+from sqlalchemy import create_engine
 
 from airflow.exceptions import AirflowException
+from airflow.providers.common.sql.hooks.sql import DbApiHook
 from airflow.providers.google.common.consts import CLIENT_INFO
-from airflow.providers.google.common.hooks.base_google import GoogleBaseHook
+from airflow.providers.google.common.hooks.base_google import GoogleBaseHook, get_field
 
 
-class SpannerHook(GoogleBaseHook):
+class SpannerConnectionParams(NamedTuple):
+    """Information about Google Spanner connection parameters."""
+
+    project_id: str | None
+    instance_id: str | None
+    database_id: str | None
+
+
+class SpannerHook(GoogleBaseHook, DbApiHook):
     """
     Hook for Google Cloud Spanner APIs.
 
     All the methods in the hook where project_id is used must be called with
     keyword arguments rather than positional.
     """
 
+    conn_name_attr = "gcp_conn_id"
+    default_conn_name = "google_cloud_spanner_default"
+    conn_type = "gcpspanner"
+    hook_name = "Google Cloud Spanner"

Review Comment:
   Agree. Extra is well, dangerous. There were many CVEs issued recently because having access to edit extras would open all kinds of ways for those who have access to edit connections to do unintended things > Remote Code Execution,  DOS, revealing extra information. We had to even reflect that in our security model and specifically flag the users who have connection editing capabilities as having to be trusted not to abuse those capabilities: https://airflow.apache.org/docs/apache-airflow/stable/security/index.html#capabilities-of-authenticated-ui-users 
   
   Generally speaking extras should be very limited and we maybe even should get rid of them at some point in time.



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