amoghrajesh commented on code in PR #62696:
URL: https://github.com/apache/airflow/pull/62696#discussion_r2922536196
##########
shared/configuration/src/airflow_shared/configuration/parser.py:
##########
@@ -166,6 +173,34 @@ def configure_parser_from_configuration_description(
parser.set(section, key, default_value)
+def create_provider_cfg_config_fallback_defaults(
+ provider_config_fallback_defaults_cfg_path: str,
+) -> ConfigParser:
+ """
+ Create fallback defaults.
Review Comment:
It might be a copypasta but worth making clearer
##########
airflow-core/tests/unit/core/test_configuration.py:
##########
@@ -64,6 +64,10 @@
"deactivate_stale_dags_interval",
"2.5.0",
)
+# Invalidate cached properties that depend on deprecated_options, since they
may have been
+# computed during airflow initialization before the entries above were added.
+for attr in ("sensitive_config_values", "inversed_deprecated_options"):
+ conf.__dict__.pop(attr, None)
Review Comment:
Can we add this on `conf` itself? Maybe a helper like
`conf._invalidate_cahe()` to clear all known cached_property entries, so test
code does not have to maintain a separate list.
##########
airflow-core/src/airflow/jobs/scheduler_job_runner.py:
##########
@@ -294,6 +299,9 @@ def __init__(
key="num_stuck_in_queued_retries",
fallback=2,
)
+ self._scheduler_use_job_schedule = conf.getboolean("scheduler",
"use_job_schedule", fallback=True)
+ self._parallelism = conf.getint("core", "parallelism")
+ self._multi_team = conf.getboolean("core", "multi_team")
Review Comment:
These values are now frozen for the lifetime of the SchedulerJobRunner as I
see it?
Prior to this change, they were read from conf on every scheduling loop
iteration, so that meant conf changes were reflected without restart? So maybe
we can instead get it in each loop body / cache it for performance?
##########
airflow-core/tests/integration/otel/test_otel.py:
##########
@@ -209,6 +209,11 @@ def setup_class(cls):
# during scheduler handoff (see
https://github.com/apache/airflow/issues/61070).
wait_for_otel_collector(otel_host, otel_port)
+ # The pytest plugin strips AIRFLOW__*__* env vars (including the JWT
secret set
+ # by Breeze). Both the scheduler and api-server subprocesses must
share the same
+ # secret; otherwise each generates its own random key and token
verification fails.
+ os.environ["AIRFLOW__API_AUTH__JWT_SECRET"] =
"test-secret-key-for-testing"
+ os.environ["AIRFLOW__API_AUTH__JWT_ISSUER"] = "airflow"
Review Comment:
Unrelated?
##########
shared/configuration/src/airflow_shared/configuration/parser.py:
##########
@@ -166,6 +173,34 @@ def configure_parser_from_configuration_description(
parser.set(section, key, default_value)
+def create_provider_cfg_config_fallback_defaults(
+ provider_config_fallback_defaults_cfg_path: str,
+) -> ConfigParser:
+ """
+ Create fallback defaults.
Review Comment:
```suggestion
Create fallback defaults for configuration.
```
##########
airflow-core/src/airflow/jobs/scheduler_job_runner.py:
##########
@@ -281,6 +281,11 @@ def __init__(
self.num_runs = num_runs
self.only_idle = only_idle
self._scheduler_idle_sleep_time = scheduler_idle_sleep_time
+
+ # Note:
+ # We need to fetch ALL the conf before the `prohibit_commit` block,
otherwise we will encounter `UNEXPECTED COMMIT - THIS WILL BREAK HA LOCKS`
since the Core conf might access the MetadataMetastoreBackend
+ # so the most easiest way is to fetch all the conf in the init method
and store them as attributes of the SchedulerJobRunner instance.
Review Comment:
Sounds good.
##########
task-sdk/src/airflow/sdk/providers_manager_runtime.py:
##########
@@ -608,6 +631,7 @@ def _cleanup(self):
self._asset_uri_handlers.clear()
self._asset_factories.clear()
self._asset_to_openlineage_converters.clear()
+ self._provider_configs.clear()
self._initialized = False
self._initialization_stack_trace = None
Review Comment:
Should we invalidate cached_property here? And in the core too?
--
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]
