potiuk commented on code in PR #32604:
URL: https://github.com/apache/airflow/pull/32604#discussion_r1266151753
##########
airflow/cli/cli_config.py:
##########
@@ -793,6 +814,13 @@ def string_lower_type(val):
action="store_true",
)
+# IMPORTANT NOTE!
+#
+# Celery configs below have explicit fallback values because celery provider
defaults are not yet loaded
+# via provider at the time we parse the command line, so in case it is not
set, we need to have manual
+# fallback
+# DO NOT REMOVE THE FALLBACKS even if you are tempted to.
+# TODO: possibly move the commands to providers but that could be big
performance hit on the CLI
Review Comment:
There is one concern I have, after my chnages, some of the commands (celery,
kubernetes ) will have to initialize ProvidersManager's
initialize_configuration() - which is going to be costly and it cannot be done
during parsing of the arguments, so I am afraid extracting them to providers
are not going to be "really" possible. I think we will have to eventually leave
the CLI definition for Celery and Kubernetes commands in the core because of
that. But I suggest we wait for it until we see it, we can likely have it
separated out, but not moved to inside provider's code.
--
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]
