potiuk commented on issue #49881:
URL: https://github.com/apache/airflow/issues/49881#issuecomment-3268462999

   > In the google provider for cloud_sql we generate a different URIs 
depending on different types of DB and then create a Connection using this 
generated URI. The absence of this parameter in the interface for Connection 
from task-sdk blocks us for smooth migration for cloud_sql hook.
   
   Actually I implemented original version of it :). Many years ago. And as I 
see it now, this is not really a big issue to refactor the method. Currently 
the method `_generate_connection_uri` does this:
   
   ```
           connection_uri = format_string.format(
               user=quote_plus(self.user) if self.user else "",
               password=quote_plus(self.password) if self.password else "",
               database=quote_plus(self.database) if self.database else "",
               public_ip=self.public_ip,
               public_port=self.public_port,
               proxy_port=self.sql_proxy_tcp_port,
               socket_path=self._quote(socket_path),
               ssl_spec=self._quote(json.dumps(ssl_spec)) if ssl_spec else "",
               client_cert_file=self._quote(self.sslcert) if self.sslcert else 
"",
               client_key_file=self._quote(self.sslkey) if self.sslcert else "",
               server_ca_file=self._quote(self.sslrootcert if self.sslcert else 
""),
           )
           self.log.info(
               "DB connection URI %s",
               connection_uri.replace(
                   quote_plus(self.password) if self.password else "PASSWORD", 
"XXXXXXXXXXXX"
               ),
           )
           return connection_uri
   ```
   
   and it's **only** used in one place (and one test)
   
   ```
           uri = self._generate_connection_uri()
           connection = Connection(conn_id=self.db_conn_id, uri=uri)
   ```
   
   
   But going through URI is not necessary - neither in Airflow 2 nor 3, you 
could build dict with kwargs and initialize Connection with it. It does not 
seem to outreagous to change the methot to:
   
   ```
   _generate_connection() -> task.sdk.Connection | model.Connection
   ```
   
   And do 
   
   * if airflow 2  -> create connection via uri
   * if aorflow 3 -> create connection via default parameters used
   
   Or even 
   
   * build kwarg dict for Connection
   * if airlfow 2 -> return models.Connection(**kwargs)
   * if airflow 3 -> return task.sdk.Connection(**kwargs)
   
   Then we could drop the Airflow 2 path when we drop Airflow 2 support.


-- 
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-unsubscr...@airflow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to