Hi all, I am against assert when it's used to replace case / error handling and is intended to be seen by end users. However, I am ok with asserts that are used to satisfy mypy. I think it's better to use "assert isinstance(x, RequiredType)" than "# type: ignore" because is more informative and in case of type changes this will rise a warning.
Bests, Tomek On Tue, Dec 3, 2019 at 9:10 PM Shaw, Damian P. < damian.sha...@credit-suisse.com> wrote: > Hi all, > > Semantic arguments aside about the meaning of assert, or the visual nicety > of raise exception vs. assert, then there is one practical difference > between assert vs. any other exception raise mechanism: the assert > statement is filtered out when generating bytecode with the "-O" flag. > > Given Airflow is so widely used I have to imagine *some* users of Airflow > use pretty much every possible configuration of starting Python. So for > these statements is the desired effect they are filtered from byte code > when a user uses the "-O" flag? > > Or asked differently, given the "-O" flag would typically be used in a > production environment to optimize code, is the intention that if a user > looking to optimize their code in production then these statements should > never run? > > From reading the code I assume the answer is no, but I may be missing > something. > > Regards > Damian > > -----Original Message----- > From: Bjorn Olsen <bjorn.ols...@gmail.com> > Sent: Tuesday, December 3, 2019 2:54 PM > To: dev@airflow.apache.org > Subject: Re: [DISCUSS] Using asserts in airflow code > > Hi all, > > This SO answer helped me on another project: > https://stackoverflow.com/a/41721518/2409299 > > The goal of an assertion in Python is to inform developers about > *unrecoverable* errors in a program. > Assertions are not intended to signal expected error conditions, like > “file not found”, where a user can take corrective action (or just try > again). > > My opinion - it's simpler to avoid asserts and rather use Exceptions. > The CPU consumed to report an Exception is negligible compared to > developer time trying to figure out an assert. > If an assert is optimized out and the condition causing it does get > triggered, then this can cause unexpected and untested behavior in the > program if it continues past where the assert was. > An Exception isn't optimised out and therefore will trigger as expected. > > Perhaps a new kind of Exception - UnrecoverableException - would be best. > However if we do decide to use asserts then it should be used sparingly, > and only for specific conditions such as above. > > Lastly, consider that it is easy to write an assert which doesn't work as > expected: > Always evaluates True: > > assert (2 + 2 == 5, "Houston we've got a problem") > > Always evaluates False: > > assert 2 + 2 == 5, "Houston we've got a problem" > > > Kind regards > > > On Tue, Dec 3, 2019 at 7:06 PM Iuliia Volkova <xnuins...@gmail.com> wrote: > > > So, now, in this code I will have no idea that is this, is this error > > from Airflow or somebody forget to remove from master code debug > > assert? So with normal error it will be like this: > > > > *assert self.futures, NOT_STARTED_MESSAGE* > > > > *if not self.futures: * > > * raise AirflowException(NOT_STARTED_MESSAGE)* > > > > second variant: more readable, does not cause any issues with any > > flags, I see in traceback what kind of error is this - some random > > Apache Airflow or maybe ValueError, or maybe TypeError - I have more > > information as developer > > > > And at the end of the day '*debug*' tool not used in production code. > > > > > > On Tue, Dec 3, 2019 at 7:53 PM Jarek Potiuk <jarek.pot...@polidea.com> > > wrote: > > > > > Just an example of such asserts which IMHO are nicer are here: > > > > > > > > https://github.com/apache/airflow/pull/6596/files#diff-4c0c36f193f2cd6 > > 5e2b55ba3102c1ba2R38 > > > One line assert with message. > > > > > > J. > > > > > > > > > > > > On Tue, Dec 3, 2019 at 5:36 PM Anton Zayniev > > > <anton.zayn...@gmail.com> > > > wrote: > > > > > > > Hi, guys. I'm really surprised about this > > > > > > > > > - (+) asserts look nicer and are more readable than if (something): > > > > > throw Exception() > > > > > > > > I'm pretty sure that all the code I have encountered a way more > > readable > > > > using "if/else" or "try/except". But may be it is just me. Could > > > > you provide an example of code which is better with "assert"? > > > > > > > > - (+) asserts are especially good for cases like None exception - > > > > they > > > > > add more developer friendly messages when they will fail a > > > > > few > > lines > > > > > below > > > > > with (for example) None has no property "dag". But it's ok if > > those > > > > get > > > > > optimised away. > > > > > > > > I think the best way to catch None is to ensure your code would > > > > fail conveniently. Like raising understandable Exception message, > > > > if you > > > believe > > > > that should be a point of confusion. > > > > > > > > On Tue, 3 Dec 2019 at 16:22, Iuliia Volkova <xnuins...@gmail.com> > > wrote: > > > > > > > > > Hi everyone, I'm usually not write anything in this mail list, > > > > > but > > this > > > > > theme something really strange > > > > > Exist offissial doc: > > > > > > > > > > > > > > https://docs.python.org/3/reference/simple_stmts.html#the-assert-state > > ment > > > > > > > > > > and there is a key information: Assert statements are a > > > > > convenient > > way > > > to > > > > > insert debugging assertions into a program. > > > > > > > > > > *Debugging. * - this is a key propose of asserts keyword. > > > > > > > > > > there is no any type of possible asserts that cannot be done > > > > > with > > > normal > > > > > Exceptions and Errors types that more explicit and detailed when > > > > 'assert' - > > > > > you have ValueError, TyperError and etc. what kind of problems > > > > > must > > > > solved > > > > > DEBUG tools in production code that can be easily turned off on > > servers > > > > by > > > > > users? > > > > > > > > > > asserts used in tests and in process of debug code, not in > > > > > production > > > > > > > > > > On Tue, Dec 3, 2019 at 6:47 PM Jarek Potiuk < > > jarek.pot...@polidea.com> > > > > > wrote: > > > > > > > > > > > We had a few discussions about using asserts in our code. I > > > > > > pasted > > > some > > > > > > links below but wanted to extract a gist of it. > > > > > > > > > > > > Here are the comments summarised: > > > > > > > > > > > > - (+) asserts look nicer and are more readable than if > > > (something): > > > > > > throw Exception() > > > > > > - (-) asserts can be optimized away with -O flag so we > > > > > > should > > not > > > > > based > > > > > > any real logic on having them > > > > > > - (+) asserts are good in cases that can happen in > > > > > > development > > but > > > > > > should "never happen" in reality > > > > > > - (+) asserts are especially good for cases like None > > > > > > exception > > - > > > > they > > > > > > add more developer friendly messages when they will fail a > > > > > > few > > > lines > > > > > > below > > > > > > with (for example) None has no property "dag". But it's ok > > > > > > if > > > those > > > > > get > > > > > > optimised away. > > > > > > > > > > > > We would like to discuss those points in community and have a > > > > community - > > > > > > driven decision on: > > > > > > > > > > > > 1) whether we should use asserts? > > > > > > 2) in which cases > > > > > > 3) in which cases we should NOT use asserts. > > > > > > > > > > > > J. > > > > > > > > > > > > The links here: > > > > > > > > > > > > Slack Discussion: > > > > > > > > > https://apache-airflow.slack.com/archives/CCQ7EGB1P/p157536466404130 > > > 0 > > > > > > > > > > > > Github threads: > > > > > > > > > > > > - > > > https://github.com/apache/airflow/pull/6596#discussion_r352916409 > > > > > > - > > > https://github.com/apache/airflow/pull/6596#discussion_r352914727 > > > > > > - > > > > > > > > > > > > https://github.com/apache/airflow/pull/3690#pullrequestreview-14337662 > > 9 > > > > > > > > > > > > Stack overflow link for asserts: > > > > > > > > > > > > - https://stackoverflow.com/a/1838411/5691525 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > J. > > > > > > > > > > > > -- > > > > > > > > > > > > Jarek Potiuk > > > > > > Polidea <https://www.polidea.com/> | Principal Software > > > > > > Engineer > > > > > > > > > > > > M: +48 660 796 129 <+48660796129> > > > > > > [image: Polidea] <https://www.polidea.com/> > > > > > > > > > > > > > > > > > > > > > -- > > > > > _________ > > > > > > > > > > С уважением, Юлия Волкова > > > > > Тел. : +7 (911) 116-71-82 > > > > > > > > > > > > > > > > > > -- > > > > > > Jarek Potiuk > > > Polidea <https://www.polidea.com/> | Principal Software Engineer > > > > > > M: +48 660 796 129 <+48660796129> > > > [image: Polidea] <https://www.polidea.com/> > > > > > > > > > -- > > _________ > > > > С уважением, Юлия Волкова > > Тел. : +7 (911) 116-71-82 > > > > > > =============================================================================== > > Please access the attached hyperlink for an important electronic > communications disclaimer: > http://www.credit-suisse.com/legal/en/disclaimer_email_ib.html > =============================================================================== > > -- Tomasz Urbaszek Polidea <https://www.polidea.com/> | Junior Software Engineer M: +48 505 628 493 <+48505628493> E: tomasz.urbas...@polidea.com <tomasz.urbasz...@polidea.com> Unique Tech Check out our projects! <https://www.polidea.com/our-work>
