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


##########
providers/src/airflow/providers/jdbc/hooks/jdbc.py:
##########
@@ -25,6 +25,7 @@
 import jaydebeapi
 import jpype
 from sqlalchemy.engine import URL
+from wrapt import synchronized

Review Comment:
   > I liked the solution as to me, coming from Java, it seemed like a nice and 
clean solution.
   
   Yeah. It's nice, but it's more of a byproduct of what they have as an 
example usage. What happens if one day they release the package and the 
"synchronized" decorator will not be there or moved? 
   
   They can do it even in patchlevel version. and claim "it's ok to move it - 
it's just an example" - we cannot rely on it being there. 
   
   We could also write our own decorator (basically copy what they have done) 
and keep it in, but that seems an overkill if we can do the same with 
`self.lock =" and `with lock` two lines added. 
   
   It's abit like infamous `leftpad` fiasco.



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