kevinalh commented on PR #38007: URL: https://github.com/apache/airflow/pull/38007#issuecomment-1987121667
> I don't think we should change order of parameters. This has a hidden potential of breaking behaviour of underlying functions > > So far we discussed with @uranusjr that better error message is actually way better behaviour. And a really I think this message should flag all cases where you pass a default value other than None to a parameter that is known to be in the context. > > This would remove all the ambiguity - because now you can pass a different default to a parameter that is name in the same way as context and we'll, it'sreally undefined what should happen. should the default be used? Should the value from context be used ? If the latter - what is the meaning of default ? > > This is simply cery inconsistent and ambiguous to pass any value as default for those parameters which is different than None. So erroring out is IMHO better solution, changing parameter order is just hiding the problem. > > @uranusjr - WDYT ? Thanks for the feedback. I agree that it will be more error-prone to change the order of parameters. I made [a different PR](https://github.com/apache/airflow/pull/38015) with the error message alternative, plus not allowing defaults other than None. Closing this one. -- 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
