potiuk commented on code in PR #24591: URL: https://github.com/apache/airflow/pull/24591#discussion_r909642422
########## 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: This is a very good point :). I don't think we are at the point yet when we "know" we will use it. It's a proposal from @blag (which also coincidentally reflect my preferences, but even with Breeze's experience I am not 100% sure if I know how click will behave with Airlfow. We use it in Breeze and well, the usability of it is great, the integration with rich is cool and it has some really good features - for example combining click, rich and rich-click has the very nice SVG export which is fantastic way of documenting CLI (and we use all of them). The story is a follows: I am personally not yet 100% sure if click is good. I think so (but I think it has never been really "thoroughly" discussed - mainly because we had no "base for dicussion I think . Some time ago we had this short "poll" (after I switched to it in Breeze) - https://lists.apache.org/thread/fcs3wwwon3wyv7bynjqvdj6nvjy5lx5l - but that was just "sentiment" of few people and I netiher planned nor did anything there and we had very little responses. Then - rather independently - @blag proposed it in https://github.com/apache/airflow/issues/22708 and I encouraged to experiment with it in https://github.com/apache/airflow/pull/24628 - but I suggested @blag to start small and get a founding PR and treat it as an experiment. And I think this PR here is really the "founding" PR that should give us some answers if this is worht or not. I think the first few (or maybe even THE FIRST) PR should give us enough answers that we should be able to discuss it. The proposal from @blag was to name the tool "airlfow-ng" and keep it as experiment in parallel to airflow. But for sure once we finish the experimentation phase there has to be a proper plan on how to migrate (if we decide to). I personally tihnk that click has big value from the user side (especially when connected with rich and rich-click) and that - when treated very carefully it can be made performant enough for us (and my comments above reflect that) that click can be made performant enough for our needs (but - as mentioned above - with the caveat of making sure that whoever contributes will not "incrementally" break the experience (hence the strong preferrence from my side on some kind of gating - also coming from our bad experiences with our "own" CLI). The worst thing I can imagine is that we switch to a new CLI that will be "way worse" due to performance issues and even worse that if some contribution will make it "gradually worse". I think this PR is perfect opportunity to see all the pros-cons and experiment with the architecture to see if the performance will be good for us and what we would need to do to make it works. For example in Breeze, we have the gate in the form of pre-commit, that simply runs auto-complete `feature` of click in a completely separate virtualenv with just rich and click installed, making sure that no other external library is imported (It fails when it tries to impoert any external package) and it already prevented us from importing some stuff like yaml (which we know is on its own too havy to be used in auto-complete). But Breeze has quite a different architecture - flat set of commands (so there is no capability of lazy-loading of sub-commands you mentioned), it does not have the "huge import chain" (I deliberately made it architected in the way that it has very little small, independent modules + it's like 100 times smaller than airflow, so I felt external deps are all that we n eed to get. I think we will not know until we try :). So I'd suggest (but that's really up to @blag how to approach it) in this small PR we shoudl explore what it is to make click works for airlfow and make it a proposal. Not sure if AIP is needed eventually (probably not) but for sure a deliberate discussion with learnings and our approach how we are going to gate the performance is needed I think -- 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]
