You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2023/01/05 10:41:26 UTC

[GitHub] [airflow] potiuk commented on a diff in pull request #28675: MongoHook optimization

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


##########
airflow/providers/mongo/hooks/mongo.py:
##########
@@ -74,12 +68,13 @@ def __exit__(
         exc_val: BaseException | None,
         exc_tb: TracebackType | None,
     ) -> None:
-        if self.client is not None:
-            self.close_conn()
+        if self.client:
+            self.client.close()
+            self.client = None
 
     def get_conn(self) -> MongoClient:
         """Fetches PyMongo Client"""
-        if self.client is not None:
+        if self.client:

Review Comment:
   No opinion. I think it's just a matter of taste. We use both aproaches across our code. There are some cases where using is None matters, here it does not. 
   
   I also think it's not worth arguing about and if a maintainer wants it to change, i'd just change it. It's far more constructive and takes far less time to comply with such request that to argue and especially to involve others to be "judges". It does not matter (in this case) if you or other person is right or not. Your goal is to merge the request, not to "win" the discussion.
   
   Really  - pick your fights wisely, otherwise you will loose too much energy (of yours and others) so that when there are things that matter, you will not have neither energy or other's willingness to listen when it really matters.



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