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 2022/11/30 21:48:32 UTC

[GitHub] [airflow] potiuk commented on pull request #28019: Use asserts instead of exceptions for executor not started

potiuk commented on PR #28019:
URL: https://github.com/apache/airflow/pull/28019#issuecomment-1332775055

   While I heartily sympathise with you, currently we agreed to not use asserts in the main code. We should only use then in tests.
   
   Initial Discussion here: https://lists.apache.org/thread/h22sodtylksn04tzm1f2n3q8h994j38p
   Voting thread here: https://lists.apache.org/thread/tl9f344vwjmzh8pqfgbjkx8rqyy7dp9n
   
   I think by disabling pylint (another vote) we lost check for it, it used to be flagged in pre-commits (and if we are going to keep the rule, we likely should add it). 
   
   As you will see, the proponent of using asserts (myself including) were out-voted, and we had no other choice by follow it (I even implemented pylint check for that). 
   
   Main reason why we (as community decided to forbid it, was potential confusion about the flows 
   
   But - maybe it's time to re-open the discussion ? I am still of an opinion that asserts in cases like you mention are rather useful. As we say - "disagree, but engage" - but if we vote again a decision is different, I am super happy to engage on the side of asserts. 


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