sjyangkevin commented on code in PR #55008:
URL: https://github.com/apache/airflow/pull/55008#discussion_r2309124885


##########
providers/mysql/src/airflow/providers/mysql/hooks/mysql.py:
##########


Review Comment:
   thanks for the proposal, and the proposed changes sounds reasonable, but 
comes with the following trade-offs.
   
   In the current implementation, the connection seems like a property. If it 
doesn't exist, a new connection will be created 
`self.get_connection(self.get_conn_id())`. Otherwise, the existing one 
`self.connection` will be reused.
   ```
   conn = self.connection or self.get_connection(self.get_conn_id())
   ```
   By making it a context manager, looks like the connection **is opened and 
closed every time** when an operation is executed, like `bulk_load` or 
`bulk_dump`. So, you might not be able to reuse the same connection for several 
operations.
   
   Also, wondering if it is compatible with the "autocommit".
   



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to