potiuk commented on code in PR #24591: URL: https://github.com/apache/airflow/pull/24591#discussion_r912005454
########## airflow/cli/__main__.py: ########## @@ -18,6 +18,7 @@ # under the License. from airflow.cli import airflow_cmd +from airflow.cli.commands import version # noqa: F401 Review Comment: I think the learning from that discussion in this PR is - click introduction is far from "done deal". Merging any PR with click changes when there are so many doubts about whether it will be performant enough. Seems that at least few of us are extremely concerned about it and seems to be something really important that can break the whole benefit click might bring and make the experience worse. Merging any click related PR makes no sense before those concerns are addressed IMHO. And we have good set of ideas that could be explored to find out an answer to the question "are we able to make click performance good enough for us". There should be Proof Of Concept that should explore those possibilities and see if this might work for us and likely an AIP describing it - trying to implement some of the most potentially troublesome cases and measuring what are the delays introduced and how we can structure the code (and possibly implement other options like mypyc) speed it up. Some studies involving the acceptable delays can be found and referred to I think. My proposal @blag - if you want to continue introducing your idea - is to turn your work (and experience you gained so far when preparing this PR and other CLI actions) into this POC that will explore those options and lead the process on proposing, discussing and voting on the AIP based on that POC. I think just merging this PR without doing this exploration makes very little sense. It's not a "scope creep" - it's just realising that scope of the orignal PR does not bring answers to more general question "do we really want to do it?" which results from the knowledge of "can we make it good enough?". I think the discussion above was valuable to show that we currently have no answer to either of those questions and merging the PR does not bring us any closer - so we should not do it. -- 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]
