potiuk commented on PR #28182: URL: https://github.com/apache/airflow/pull/28182#issuecomment-1356907529
And just to explain a bit - I realised closing the issue could have sounded a bit rude. But it was not really and in my mind it was pretty clear at that time (and this is how I interpreted the title of the PR and the "oops" + opening another issue by TP. But I really thought the excessive code - especially the `getattr` to provide backwards compatibility was totally not worth it. Effectively the trade-off was: a)const definition stays vs. b) importing regexp library + adding getattr method in which the same constant was effectively repeated, adding new method where not only logic for gettattr + adding tests if regexp works + message that required the user to parse the regexp The title of the PR was "better message" which was not too much better (telling regexp in a user message is a sign that we expect the user can parse the regexp, which is kinda non-human. Simply speaking the change from TP looked so much more doing what your PR title suggested - "WAY better message". There could be other reasons why we could get rid of the constant, but I think just "better user message" is not one of those reasons. Sorry if you took that as being rude or smth like that. It was just perfectly logical choice for me. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
