Automatically create section when migrating config

[uranusjr]

Previously, if a config is migrated to a new section, the migration code would crash with NoSectionError if the user does not add that section to airflow.cfg after upgrading Airflow. This patch automatically creates an empty section when that happens to avoid Airflow from crashing.

[kaxil]

Previously, if a config is migrated to a new section, the migration code would crash with NoSectionError if the user does not add that section to airflow.cfg after upgrading Airflow. This patch automatically creates an empty section when that happens to avoid Airflow from crashing.

It should not error, as we have deprecations for the configs/sections that are moved:

airflow/airflow/configuration.py

Lines 105 to 185 in b251d22

	class AirflowConfigParser(ConfigParser):
	"""Custom Airflow Configparser supporting defaults and deprecated options"""
	# These configuration elements can be fetched as the stdout of commands
	# following the "{section}__{name}__cmd" pattern, the idea behind this
	# is to not store password on boxes in text files.
	# These configs can also be fetched from Secrets backend
	# following the "{section}__{name}__secret" pattern
	sensitive_config_values = {
	('core', 'sql_alchemy_conn'),
	('core', 'fernet_key'),
	('celery', 'broker_url'),
	('celery', 'flower_basic_auth'),
	('celery', 'result_backend'),
	('atlas', 'password'),
	('smtp', 'smtp_password'),
	('webserver', 'secret_key'),
	}
	# A mapping of (new section, new option) -> (old section, old option, since_version).
	# When reading new option, the old option will be checked to see if it exists. If it does a
	# DeprecationWarning will be issued and the old option will be used instead
	deprecated_options = {
	('celery', 'worker_precheck'): ('core', 'worker_precheck', '2.0.0'),
	('logging', 'base_log_folder'): ('core', 'base_log_folder', '2.0.0'),
	('logging', 'remote_logging'): ('core', 'remote_logging', '2.0.0'),
	('logging', 'remote_log_conn_id'): ('core', 'remote_log_conn_id', '2.0.0'),
	('logging', 'remote_base_log_folder'): ('core', 'remote_base_log_folder', '2.0.0'),
	('logging', 'encrypt_s3_logs'): ('core', 'encrypt_s3_logs', '2.0.0'),
	('logging', 'logging_level'): ('core', 'logging_level', '2.0.0'),
	('logging', 'fab_logging_level'): ('core', 'fab_logging_level', '2.0.0'),
	('logging', 'logging_config_class'): ('core', 'logging_config_class', '2.0.0'),
	('logging', 'colored_console_log'): ('core', 'colored_console_log', '2.0.0'),
	('logging', 'colored_log_format'): ('core', 'colored_log_format', '2.0.0'),
	('logging', 'colored_formatter_class'): ('core', 'colored_formatter_class', '2.0.0'),
	('logging', 'log_format'): ('core', 'log_format', '2.0.0'),
	('logging', 'simple_log_format'): ('core', 'simple_log_format', '2.0.0'),
	('logging', 'task_log_prefix_template'): ('core', 'task_log_prefix_template', '2.0.0'),
	('logging', 'log_filename_template'): ('core', 'log_filename_template', '2.0.0'),
	('logging', 'log_processor_filename_template'): ('core', 'log_processor_filename_template', '2.0.0'),
	('logging', 'dag_processor_manager_log_location'): (
	'core',
	'dag_processor_manager_log_location',
	'2.0.0',
	),
	('logging', 'task_log_reader'): ('core', 'task_log_reader', '2.0.0'),
	('metrics', 'statsd_on'): ('scheduler', 'statsd_on', '2.0.0'),
	('metrics', 'statsd_host'): ('scheduler', 'statsd_host', '2.0.0'),
	('metrics', 'statsd_port'): ('scheduler', 'statsd_port', '2.0.0'),
	('metrics', 'statsd_prefix'): ('scheduler', 'statsd_prefix', '2.0.0'),
	('metrics', 'statsd_allow_list'): ('scheduler', 'statsd_allow_list', '2.0.0'),
	('metrics', 'stat_name_handler'): ('scheduler', 'stat_name_handler', '2.0.0'),
	('metrics', 'statsd_datadog_enabled'): ('scheduler', 'statsd_datadog_enabled', '2.0.0'),
	('metrics', 'statsd_datadog_tags'): ('scheduler', 'statsd_datadog_tags', '2.0.0'),
	('metrics', 'statsd_custom_client_path'): ('scheduler', 'statsd_custom_client_path', '2.0.0'),
	('scheduler', 'parsing_processes'): ('scheduler', 'max_threads', '1.10.14'),
	('operators', 'default_queue'): ('celery', 'default_queue', '2.1.0'),
	('core', 'hide_sensitive_var_conn_fields'): ('admin', 'hide_sensitive_variable_fields', '2.1.0'),
	('core', 'sensitive_var_conn_names'): ('admin', 'sensitive_variable_fields', '2.1.0'),
	('core', 'default_pool_task_slot_count'): ('core', 'non_pooled_task_slot_count', '1.10.4'),
	('core', 'max_active_tasks_per_dag'): ('core', 'dag_concurrency', '2.2.0'),
	}
	# A mapping of old default values that we want to change and warn the user
	# about. Mapping of section -> setting -> { old, replace, by_version }
	deprecated_values = {
	'core': {
	'hostname_callable': (re.compile(r':'), r'.', '2.1'),
	},
	'webserver': {
	'navbar_color': (re.compile(r'\A#007A87\Z', re.IGNORECASE), '#fff', '2.1'),
	},
	'email': {
	'email_backend': (
	re.compile(r'^airflow\.contrib\.utils\.sendgrid\.send_email$'),
	r'airflow.providers.sendgrid.utils.emailer.send_email',
	'2.1',
	),
	},
	}

[kaxil]

And for the all configs that don't exist in user's airflow.cfg we default to Airflow's default config (which is separate than user's airflow.cfg)

airflow/airflow/configuration.py

Line 359 in b251d22

return self._get_option_from_default_config(section, key, **kwargs)

[ashb]

@kaxil Ohh, I see.

So the section exists in the default_config, but when we to the upgrade and try to write the config back , the sections are only checked in the top level config, cos that's where we are writing to.

[ashb]

@uranusjr We should probably expand the unit tests to cover this case

[ashb]

@uranusjr Ping - I think this is worth finishing off.

[uranusjr]

Trying to figure out what needs to be done here. So the patch is not wrong (although my description to the issue was, and a unit test should be added to cover the section-only-in-default-config-not-user-config case. Is that right?

[ashb]

and a unit test should be added to cover the section-only-in-default-config-not-user-config case. Is that right?

Yup, that's right.

[github-actions]

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.