amoghrajesh commented on code in PR #32604:
URL: https://github.com/apache/airflow/pull/32604#discussion_r1269241851
##########
airflow/configuration.py:
##########
@@ -1732,6 +1787,31 @@ def
create_default_config_parser(configuration_description: dict[str, dict[str,
return parser
+def write_default_airflow_configuration_if_needed():
+ if not os.path.isfile(AIRFLOW_CONFIG):
+ import rich
+
+ rich.print("Creating new Airflow config file in: %s", AIRFLOW_CONFIG)
Review Comment:
from rich import print here?
##########
airflow/cli/cli_config.py:
##########
@@ -818,6 +823,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:
Should we rather have it at the top of the page or elsewhere?
##########
airflow/configuration.py:
##########
@@ -151,15 +151,32 @@ def _default_config_file_path(file_name: str) -> str:
return os.path.join(templates_dir, file_name)
-def retrieve_configuration_description() -> dict[str, dict[str, Any]]:
+def retrieve_configuration_description(
+ include_airflow: bool = True,
+ include_providers: bool = True,
+ selected_provider: str | None = None,
+ config_file_name: str = "config.yml",
+) -> dict[str, dict[str, Any]]:
"""
Read Airflow configuration description from YAML file.
+ :param include_airflow: Include Airflow configs
+ :param include_providers: Include provider configs
+ :param selected_provider: If specified, include selected provider only
+ :param config_file_name: name of the file in "config_templates" directory
to read default config from
:return: Python dictionary containing configs & their info
"""
base_configuration_description: dict[str, dict[str, Any]] = {}
- with open(_default_config_file_path("config.yml")) as config_file:
- base_configuration_description.update(yaml.safe_load(config_file))
+ if include_airflow:
+ with open(_default_config_file_path("config.yml")) as config_file:
+ base_configuration_description.update(yaml.safe_load(config_file))
+ if include_providers:
+ from airflow.providers_manager import ProvidersManager
+
+ for provider, config in ProvidersManager().provider_configs:
+ if selected_provider and provider != selected_provider:
+ continue
+ base_configuration_description.update(config)
Review Comment:
Is it a valid scenario to include both together? Or should it be an if-else
if
##########
docs/apache-airflow/core-concepts/executor/celery_kubernetes.rst:
##########
@@ -28,6 +28,8 @@ An executor is chosen to run a task based on the task's queue.
``CeleryKubernetesExecutor`` inherits the scalability of the
``CeleryExecutor`` to
handle the high load at the peak time and runtime isolation of the
``KubernetesExecutor``.
+The configuration parameters of the Celery Executor can be found in
:doc:`apache-airflow-providers-celery:configurations-ref`.
Review Comment:
Should this be a note instead?
##########
airflow/cli/commands/config_command.py:
##########
@@ -48,6 +52,9 @@ def show_config(args):
def get_value(args):
"""Get one value from configuration."""
+ from airflow.providers_manager import ProvidersManager
+
+ ProvidersManager().initialize_providers_configuration()
Review Comment:
Is `initialize_providers_configuration` an idempotent operation, why do we
need to add it here?
##########
airflow/providers/celery/executors/celery_kubernetes_executor.py:
##########
@@ -55,14 +56,21 @@ class CeleryKubernetesExecutor(LoggingMixin):
callback_sink: BaseCallbackSink | None = None
- KUBERNETES_QUEUE = conf.get("celery_kubernetes_executor",
"kubernetes_queue")
+ @cached_property
+ def kubernetes_queue(self) -> str:
+ # lazily retrieve the value of kubernetes_queue from the configuration
+ # because it might need providers
+ from airflow.providers_manager import ProvidersManager
+
+ ProvidersManager().initialize_providers_configuration()
+ return conf.get("celery_kubernetes_executor", "kubernetes_queue")
Review Comment:
Add a fallback here?
##########
docs/apache-airflow-providers/index.rst:
##########
@@ -75,7 +83,7 @@ Extra links
The providers can add extra custom links to operators delivered by the
provider. Those will be visible in
task details view of the task.
-You can see all the extra links available via community-managed providers in
+You can see all extra links available via community-managed providers in
Review Comment:
super nit:
```suggestion
You can see all the extra links available via community-managed providers in
```
##########
docs/apache-airflow/core-concepts/executor/celery.rst:
##########
@@ -29,6 +29,8 @@ change your ``airflow.cfg`` to point the executor parameter to
For more information about setting up a Celery broker, refer to the
exhaustive `Celery documentation on the topic
<https://docs.celeryq.dev/en/latest/getting-started/>`_.
+The configuration parameters of the Celery Executor can be found in
:doc:`apache-airflow-providers-celery:configurations-ref`.
Review Comment:
Should these be notes instead?
##########
docs/apache-airflow-providers/index.rst:
##########
@@ -48,16 +48,24 @@ describe all the custom capabilities.
Airflow automatically discovers which providers add those additional
capabilities and, once you install
provider package and re-start Airflow, those become automatically available to
Airflow Users.
-The summary of the core functionalities that can be extended are available in
+The summary of all core functionalities that can be extended are available in
:doc:`/core-extensions/index`.
+Configuration
+'''''''''''''
+
+Providers can have their own configuration options which allow you to
configure how they work:
+
+You can see all community-managed providers with their own configuration in
+:doc:`/core-extensions/configurations`
+
Auth backends
'''''''''''''
The providers can add custom authentication backends, that allow you to
configure the way how your
web server authenticates your users, integrating it with public or private
authentication services.
-You can see all the authentication backends available via community-managed
providers in
+You can see all authentication backends available via community-managed
providers in
Review Comment:
super nit:
```suggestion
You can see all the authentication backends available via community-managed
providers in
```
##########
docs/exts/configuration.rst.jinja2:
##########
@@ -0,0 +1,21 @@
+{#
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+#}
+{%for provider_package_name in items %}
+* :doc:`Configuration for {{ provider_package_name }} <{{
provider_package_name }}:configurations-ref>`
+{% endfor %}
Review Comment:
Wow didnt know this way possible!
--
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]
