Drop the LogTemplate DB model - #69520

Closed
jason810496 wants to merge 1 commit into
apache:mainfrom
jason810496:refactor/logging/drop-log-template-model
Closed

Drop the LogTemplate DB model#69520
jason810496 wants to merge 1 commit into
apache:mainfrom
jason810496:refactor/logging/drop-log-template-model

Conversation

@jason810496

@jason810496jason810496 commented Jul 7, 2026

Copy link
Copy Markdown
Member

Why

The LogTemplate model pinned historical values of [logging] log_filename_template / [elasticsearch] log_id_template per DagRun, and task handlers read it via dag_run.get_log_template(session=...) — a direct metadata-DB dependency that conflicts with the Airflow 3 execution model and couples log handlers to an Airflow-core model just to read two strings already available from config.

Airflow 2.11.1 already disabled per-run template retrieval by default for security reasons (#61880); this completes that direction on main by removing the model entirely.

What

  • Remove the LogTemplate model, the dag_run.log_template_id column/FK, DagRun.get_log_template(), and synchronize_log_template() with its call sites. FileTaskHandler._render_filename() now reads [logging] log_filename_template from live config at render time.
  • Add migration 0126_3_4_0_drop_log_template.py (downgrade recreates the column and table); both directions use disable_sqlite_fkeys(op) so the SQLite dag_run rebuild doesn't cascade-delete task_instance rows.
  • Elasticsearch: remove the USE_PER_RUN_LOG_ID flag — defined but unused since ElasticsearchRemoteLogIO, so dead-code cleanup only.
  • OpenSearch: keep the hasattr(DagRun, "get_log_template") shim — per-run pinning must keep working on cores 3.0-3.3; on 3.4.0+ it self-disables and the handler falls back to [opensearch] log_id_template from config.
  • Make the shared create_log_template fixture version-adaptive so provider tests pass against all supported cores; update ES docs and add a significant-change newsfragment.

Behaviour change (core 3.4.0+): changing log_filename_template / log_id_template now applies immediately and retroactively to all DagRuns — there is no more per-run historical pinning, so logs written under a previous template are no longer reachable through the new one.


Was generative AI tooling used to co-author this PR?

@jason810496jason810496 self-assigned this Jul 7, 2026
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 04e801f to 81fa395CompareJuly 7, 2026 05:36
@jason810496jason810496 added the full tests needed We need to run full set of tests for this PR to merge label Jul 7, 2026
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 81fa395 to 1984f03CompareJuly 7, 2026 11:12
The ES/OpenSearch task handlers reached into the metadata DB directly
to read the per-DagRun log template, which conflicts with the Airflow 3
execution model where task handlers shouldn't query the DB directly.
Log filename/ID templates are now always read from live config instead
of being pinned to the value in effect when a DagRun was created.
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 1984f03 to 351b9adCompareJuly 9, 2026 05:20
@jason810496
jason810496 marked this pull request as ready for review July 9, 2026 05:22
@jason810496
jason810496 requested review from kaxil and potiukJuly 9, 2026 05:22
@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

The entire reason this exists is so that if the config changes the old logs are still viewable. This breaks that doesn't it

@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

a direct metadata-DB dependency that conflicts with the Airflow 3 execution model

This isn't true - we already handle this correctly since 3.0.0 by passing the log file suffix to the worker in ExecuteTaskWorkload; changes in 2.11 directly aren't available in 3.x either so that linked pr isn't really relevant

@jason810496

jason810496 commented Jul 9, 2026

Copy link
Copy Markdown
MemberAuthor

The entire reason this exists is so that if the config changes the old logs are still viewable. This breaks that doesn't it

Yes, but based on

  • a) Airflow 2.11.1 already disabled per-run template retrieval by default for security reasons (https://github.com/apache/airflow/pull/61880);
  • b) the ES / OS provider for Airflow 3 did respect the log_template (more details as below)

I thought we can introduce the breaking change in 3.4 to drop this feature.

a direct metadata-DB dependency that conflicts with the Airflow 3 execution model

This isn't true - we already handle this correctly since 3.0.0 by passing the log file suffix to the worker in ExecuteTaskWorkload; changes in 2.11 directly aren't available in 3.x either so that linked pr isn't really relevant

It's different part, not for the log file suffix, only the ES / OS consumed the LogTemplate DB model.

IIRC, the ES / OS provider (mainly #53821) that works with Airflow 3 didn't respect the log_template model (both write and read side will fetch the latest config value instead of going through the LogTemplate model. When accessing the log_template property on TI, it hit the prohibited commit exception, so we workaround as directly accessing the conf.) at all (I will double check for this part) this is the reason why I raise the discussion to drop the LogTemplate DB model.

Or another direction, I can make the OS and ES provider still respect the LogTemplate DB model (introducing LogTemplate accessor, or retrieve the LogTemplate at Execution API and set as the StartUpDetails, etc)
WDYT? Thanks.

@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

LogTemplate is used well beyond just ES -- it's used to generate the log file suffix that is passed to workers where they write the local log file, and for reading old task logs back from S3/GCS etc.

@jason810496

Copy link
Copy Markdown
MemberAuthor

LogTemplate is used well beyond just ES -- it's used to generate the log file suffix that is passed to workers where they write the local log file, and for reading old task logs back from S3/GCS etc.

Thanks for the clarification. I see, then I will go through the flow again then restoring the LogTemplate for ES should be the correct direction, thanks.

@jason810496

Copy link
Copy Markdown
MemberAuthor

Dropping the LogTemplate DB model is the wrong direction, create #69688 to resolve the root cause.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@jason810496@ashb
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Drop the LogTemplate DB model - #69520

Closed
jason810496 wants to merge 1 commit into
apache:mainfrom
jason810496:refactor/logging/drop-log-template-model
Closed

Drop the LogTemplate DB model#69520
jason810496 wants to merge 1 commit into
apache:mainfrom
jason810496:refactor/logging/drop-log-template-model

Conversation

@jason810496

@jason810496jason810496 commented Jul 7, 2026

Copy link
Copy Markdown
Member

Why

The LogTemplate model pinned historical values of [logging] log_filename_template / [elasticsearch] log_id_template per DagRun, and task handlers read it via dag_run.get_log_template(session=...) — a direct metadata-DB dependency that conflicts with the Airflow 3 execution model and couples log handlers to an Airflow-core model just to read two strings already available from config.

Airflow 2.11.1 already disabled per-run template retrieval by default for security reasons (#61880); this completes that direction on main by removing the model entirely.

What

  • Remove the LogTemplate model, the dag_run.log_template_id column/FK, DagRun.get_log_template(), and synchronize_log_template() with its call sites. FileTaskHandler._render_filename() now reads [logging] log_filename_template from live config at render time.
  • Add migration 0126_3_4_0_drop_log_template.py (downgrade recreates the column and table); both directions use disable_sqlite_fkeys(op) so the SQLite dag_run rebuild doesn't cascade-delete task_instance rows.
  • Elasticsearch: remove the USE_PER_RUN_LOG_ID flag — defined but unused since ElasticsearchRemoteLogIO, so dead-code cleanup only.
  • OpenSearch: keep the hasattr(DagRun, "get_log_template") shim — per-run pinning must keep working on cores 3.0-3.3; on 3.4.0+ it self-disables and the handler falls back to [opensearch] log_id_template from config.
  • Make the shared create_log_template fixture version-adaptive so provider tests pass against all supported cores; update ES docs and add a significant-change newsfragment.

Behaviour change (core 3.4.0+): changing log_filename_template / log_id_template now applies immediately and retroactively to all DagRuns — there is no more per-run historical pinning, so logs written under a previous template are no longer reachable through the new one.


Was generative AI tooling used to co-author this PR?

@jason810496jason810496 self-assigned this Jul 7, 2026
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 04e801f to 81fa395CompareJuly 7, 2026 05:36
@jason810496jason810496 added the full tests needed We need to run full set of tests for this PR to merge label Jul 7, 2026
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 81fa395 to 1984f03CompareJuly 7, 2026 11:12
The ES/OpenSearch task handlers reached into the metadata DB directly
to read the per-DagRun log template, which conflicts with the Airflow 3
execution model where task handlers shouldn't query the DB directly.
Log filename/ID templates are now always read from live config instead
of being pinned to the value in effect when a DagRun was created.
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 1984f03 to 351b9adCompareJuly 9, 2026 05:20
@jason810496
jason810496 marked this pull request as ready for review July 9, 2026 05:22
@jason810496
jason810496 requested review from kaxil and potiukJuly 9, 2026 05:22
@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

The entire reason this exists is so that if the config changes the old logs are still viewable. This breaks that doesn't it

@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

a direct metadata-DB dependency that conflicts with the Airflow 3 execution model

This isn't true - we already handle this correctly since 3.0.0 by passing the log file suffix to the worker in ExecuteTaskWorkload; changes in 2.11 directly aren't available in 3.x either so that linked pr isn't really relevant

@jason810496

jason810496 commented Jul 9, 2026

Copy link
Copy Markdown
MemberAuthor

The entire reason this exists is so that if the config changes the old logs are still viewable. This breaks that doesn't it

Yes, but based on

  • a) Airflow 2.11.1 already disabled per-run template retrieval by default for security reasons (https://github.com/apache/airflow/pull/61880);
  • b) the ES / OS provider for Airflow 3 did respect the log_template (more details as below)

I thought we can introduce the breaking change in 3.4 to drop this feature.

a direct metadata-DB dependency that conflicts with the Airflow 3 execution model

This isn't true - we already handle this correctly since 3.0.0 by passing the log file suffix to the worker in ExecuteTaskWorkload; changes in 2.11 directly aren't available in 3.x either so that linked pr isn't really relevant

It's different part, not for the log file suffix, only the ES / OS consumed the LogTemplate DB model.

IIRC, the ES / OS provider (mainly #53821) that works with Airflow 3 didn't respect the log_template model (both write and read side will fetch the latest config value instead of going through the LogTemplate model. When accessing the log_template property on TI, it hit the prohibited commit exception, so we workaround as directly accessing the conf.) at all (I will double check for this part) this is the reason why I raise the discussion to drop the LogTemplate DB model.

Or another direction, I can make the OS and ES provider still respect the LogTemplate DB model (introducing LogTemplate accessor, or retrieve the LogTemplate at Execution API and set as the StartUpDetails, etc)
WDYT? Thanks.

@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

LogTemplate is used well beyond just ES -- it's used to generate the log file suffix that is passed to workers where they write the local log file, and for reading old task logs back from S3/GCS etc.

@jason810496

Copy link
Copy Markdown
MemberAuthor

LogTemplate is used well beyond just ES -- it's used to generate the log file suffix that is passed to workers where they write the local log file, and for reading old task logs back from S3/GCS etc.

Thanks for the clarification. I see, then I will go through the flow again then restoring the LogTemplate for ES should be the correct direction, thanks.

@jason810496

Copy link
Copy Markdown
MemberAuthor

Dropping the LogTemplate DB model is the wrong direction, create #69688 to resolve the root cause.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@jason810496@ashb
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Drop the LogTemplate DB model - #69520

Closed
jason810496 wants to merge 1 commit into
apache:mainfrom
jason810496:refactor/logging/drop-log-template-model
Closed

Drop the LogTemplate DB model#69520
jason810496 wants to merge 1 commit into
apache:mainfrom
jason810496:refactor/logging/drop-log-template-model

Conversation

@jason810496

@jason810496jason810496 commented Jul 7, 2026

Copy link
Copy Markdown
Member

Why

The LogTemplate model pinned historical values of [logging] log_filename_template / [elasticsearch] log_id_template per DagRun, and task handlers read it via dag_run.get_log_template(session=...) — a direct metadata-DB dependency that conflicts with the Airflow 3 execution model and couples log handlers to an Airflow-core model just to read two strings already available from config.

Airflow 2.11.1 already disabled per-run template retrieval by default for security reasons (#61880); this completes that direction on main by removing the model entirely.

What

  • Remove the LogTemplate model, the dag_run.log_template_id column/FK, DagRun.get_log_template(), and synchronize_log_template() with its call sites. FileTaskHandler._render_filename() now reads [logging] log_filename_template from live config at render time.
  • Add migration 0126_3_4_0_drop_log_template.py (downgrade recreates the column and table); both directions use disable_sqlite_fkeys(op) so the SQLite dag_run rebuild doesn't cascade-delete task_instance rows.
  • Elasticsearch: remove the USE_PER_RUN_LOG_ID flag — defined but unused since ElasticsearchRemoteLogIO, so dead-code cleanup only.
  • OpenSearch: keep the hasattr(DagRun, "get_log_template") shim — per-run pinning must keep working on cores 3.0-3.3; on 3.4.0+ it self-disables and the handler falls back to [opensearch] log_id_template from config.
  • Make the shared create_log_template fixture version-adaptive so provider tests pass against all supported cores; update ES docs and add a significant-change newsfragment.

Behaviour change (core 3.4.0+): changing log_filename_template / log_id_template now applies immediately and retroactively to all DagRuns — there is no more per-run historical pinning, so logs written under a previous template are no longer reachable through the new one.


Was generative AI tooling used to co-author this PR?

@jason810496jason810496 self-assigned this Jul 7, 2026
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 04e801f to 81fa395CompareJuly 7, 2026 05:36
@jason810496jason810496 added the full tests needed We need to run full set of tests for this PR to merge label Jul 7, 2026
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 81fa395 to 1984f03CompareJuly 7, 2026 11:12
The ES/OpenSearch task handlers reached into the metadata DB directly
to read the per-DagRun log template, which conflicts with the Airflow 3
execution model where task handlers shouldn't query the DB directly.
Log filename/ID templates are now always read from live config instead
of being pinned to the value in effect when a DagRun was created.
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 1984f03 to 351b9adCompareJuly 9, 2026 05:20
@jason810496
jason810496 marked this pull request as ready for review July 9, 2026 05:22
@jason810496
jason810496 requested review from kaxil and potiukJuly 9, 2026 05:22
@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

The entire reason this exists is so that if the config changes the old logs are still viewable. This breaks that doesn't it

@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

a direct metadata-DB dependency that conflicts with the Airflow 3 execution model

This isn't true - we already handle this correctly since 3.0.0 by passing the log file suffix to the worker in ExecuteTaskWorkload; changes in 2.11 directly aren't available in 3.x either so that linked pr isn't really relevant

@jason810496

jason810496 commented Jul 9, 2026

Copy link
Copy Markdown
MemberAuthor

The entire reason this exists is so that if the config changes the old logs are still viewable. This breaks that doesn't it

Yes, but based on

  • a) Airflow 2.11.1 already disabled per-run template retrieval by default for security reasons (https://github.com/apache/airflow/pull/61880);
  • b) the ES / OS provider for Airflow 3 did respect the log_template (more details as below)

I thought we can introduce the breaking change in 3.4 to drop this feature.

a direct metadata-DB dependency that conflicts with the Airflow 3 execution model

This isn't true - we already handle this correctly since 3.0.0 by passing the log file suffix to the worker in ExecuteTaskWorkload; changes in 2.11 directly aren't available in 3.x either so that linked pr isn't really relevant

It's different part, not for the log file suffix, only the ES / OS consumed the LogTemplate DB model.

IIRC, the ES / OS provider (mainly #53821) that works with Airflow 3 didn't respect the log_template model (both write and read side will fetch the latest config value instead of going through the LogTemplate model. When accessing the log_template property on TI, it hit the prohibited commit exception, so we workaround as directly accessing the conf.) at all (I will double check for this part) this is the reason why I raise the discussion to drop the LogTemplate DB model.

Or another direction, I can make the OS and ES provider still respect the LogTemplate DB model (introducing LogTemplate accessor, or retrieve the LogTemplate at Execution API and set as the StartUpDetails, etc)
WDYT? Thanks.

@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

LogTemplate is used well beyond just ES -- it's used to generate the log file suffix that is passed to workers where they write the local log file, and for reading old task logs back from S3/GCS etc.

@jason810496

Copy link
Copy Markdown
MemberAuthor

LogTemplate is used well beyond just ES -- it's used to generate the log file suffix that is passed to workers where they write the local log file, and for reading old task logs back from S3/GCS etc.

Thanks for the clarification. I see, then I will go through the flow again then restoring the LogTemplate for ES should be the correct direction, thanks.

@jason810496

Copy link
Copy Markdown
MemberAuthor

Dropping the LogTemplate DB model is the wrong direction, create #69688 to resolve the root cause.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@jason810496@ashb
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Drop the LogTemplate DB model - #69520

Closed
jason810496 wants to merge 1 commit into
apache:mainfrom
jason810496:refactor/logging/drop-log-template-model
Closed

Drop the LogTemplate DB model#69520
jason810496 wants to merge 1 commit into
apache:mainfrom
jason810496:refactor/logging/drop-log-template-model

Conversation

@jason810496

@jason810496jason810496 commented Jul 7, 2026

Copy link
Copy Markdown
Member

Why

The LogTemplate model pinned historical values of [logging] log_filename_template / [elasticsearch] log_id_template per DagRun, and task handlers read it via dag_run.get_log_template(session=...) — a direct metadata-DB dependency that conflicts with the Airflow 3 execution model and couples log handlers to an Airflow-core model just to read two strings already available from config.

Airflow 2.11.1 already disabled per-run template retrieval by default for security reasons (#61880); this completes that direction on main by removing the model entirely.

What

  • Remove the LogTemplate model, the dag_run.log_template_id column/FK, DagRun.get_log_template(), and synchronize_log_template() with its call sites. FileTaskHandler._render_filename() now reads [logging] log_filename_template from live config at render time.
  • Add migration 0126_3_4_0_drop_log_template.py (downgrade recreates the column and table); both directions use disable_sqlite_fkeys(op) so the SQLite dag_run rebuild doesn't cascade-delete task_instance rows.
  • Elasticsearch: remove the USE_PER_RUN_LOG_ID flag — defined but unused since ElasticsearchRemoteLogIO, so dead-code cleanup only.
  • OpenSearch: keep the hasattr(DagRun, "get_log_template") shim — per-run pinning must keep working on cores 3.0-3.3; on 3.4.0+ it self-disables and the handler falls back to [opensearch] log_id_template from config.
  • Make the shared create_log_template fixture version-adaptive so provider tests pass against all supported cores; update ES docs and add a significant-change newsfragment.

Behaviour change (core 3.4.0+): changing log_filename_template / log_id_template now applies immediately and retroactively to all DagRuns — there is no more per-run historical pinning, so logs written under a previous template are no longer reachable through the new one.


Was generative AI tooling used to co-author this PR?

@jason810496jason810496 self-assigned this Jul 7, 2026
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 04e801f to 81fa395CompareJuly 7, 2026 05:36
@jason810496jason810496 added the full tests needed We need to run full set of tests for this PR to merge label Jul 7, 2026
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 81fa395 to 1984f03CompareJuly 7, 2026 11:12
The ES/OpenSearch task handlers reached into the metadata DB directly
to read the per-DagRun log template, which conflicts with the Airflow 3
execution model where task handlers shouldn't query the DB directly.
Log filename/ID templates are now always read from live config instead
of being pinned to the value in effect when a DagRun was created.
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 1984f03 to 351b9adCompareJuly 9, 2026 05:20
@jason810496
jason810496 marked this pull request as ready for review July 9, 2026 05:22
@jason810496
jason810496 requested review from kaxil and potiukJuly 9, 2026 05:22
@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

The entire reason this exists is so that if the config changes the old logs are still viewable. This breaks that doesn't it

@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

a direct metadata-DB dependency that conflicts with the Airflow 3 execution model

This isn't true - we already handle this correctly since 3.0.0 by passing the log file suffix to the worker in ExecuteTaskWorkload; changes in 2.11 directly aren't available in 3.x either so that linked pr isn't really relevant

@jason810496

jason810496 commented Jul 9, 2026

Copy link
Copy Markdown
MemberAuthor

The entire reason this exists is so that if the config changes the old logs are still viewable. This breaks that doesn't it

Yes, but based on

  • a) Airflow 2.11.1 already disabled per-run template retrieval by default for security reasons (https://github.com/apache/airflow/pull/61880);
  • b) the ES / OS provider for Airflow 3 did respect the log_template (more details as below)

I thought we can introduce the breaking change in 3.4 to drop this feature.

a direct metadata-DB dependency that conflicts with the Airflow 3 execution model

This isn't true - we already handle this correctly since 3.0.0 by passing the log file suffix to the worker in ExecuteTaskWorkload; changes in 2.11 directly aren't available in 3.x either so that linked pr isn't really relevant

It's different part, not for the log file suffix, only the ES / OS consumed the LogTemplate DB model.

IIRC, the ES / OS provider (mainly #53821) that works with Airflow 3 didn't respect the log_template model (both write and read side will fetch the latest config value instead of going through the LogTemplate model. When accessing the log_template property on TI, it hit the prohibited commit exception, so we workaround as directly accessing the conf.) at all (I will double check for this part) this is the reason why I raise the discussion to drop the LogTemplate DB model.

Or another direction, I can make the OS and ES provider still respect the LogTemplate DB model (introducing LogTemplate accessor, or retrieve the LogTemplate at Execution API and set as the StartUpDetails, etc)
WDYT? Thanks.

@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

LogTemplate is used well beyond just ES -- it's used to generate the log file suffix that is passed to workers where they write the local log file, and for reading old task logs back from S3/GCS etc.

@jason810496

Copy link
Copy Markdown
MemberAuthor

LogTemplate is used well beyond just ES -- it's used to generate the log file suffix that is passed to workers where they write the local log file, and for reading old task logs back from S3/GCS etc.

Thanks for the clarification. I see, then I will go through the flow again then restoring the LogTemplate for ES should be the correct direction, thanks.

@jason810496

Copy link
Copy Markdown
MemberAuthor

Dropping the LogTemplate DB model is the wrong direction, create #69688 to resolve the root cause.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@jason810496@ashb
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Drop the LogTemplate DB model - #69520

Closed
jason810496 wants to merge 1 commit into
apache:mainfrom
jason810496:refactor/logging/drop-log-template-model
Closed

Drop the LogTemplate DB model#69520
jason810496 wants to merge 1 commit into
apache:mainfrom
jason810496:refactor/logging/drop-log-template-model

Conversation

@jason810496

@jason810496jason810496 commented Jul 7, 2026

Copy link
Copy Markdown
Member

Why

The LogTemplate model pinned historical values of [logging] log_filename_template / [elasticsearch] log_id_template per DagRun, and task handlers read it via dag_run.get_log_template(session=...) — a direct metadata-DB dependency that conflicts with the Airflow 3 execution model and couples log handlers to an Airflow-core model just to read two strings already available from config.

Airflow 2.11.1 already disabled per-run template retrieval by default for security reasons (#61880); this completes that direction on main by removing the model entirely.

What

  • Remove the LogTemplate model, the dag_run.log_template_id column/FK, DagRun.get_log_template(), and synchronize_log_template() with its call sites. FileTaskHandler._render_filename() now reads [logging] log_filename_template from live config at render time.
  • Add migration 0126_3_4_0_drop_log_template.py (downgrade recreates the column and table); both directions use disable_sqlite_fkeys(op) so the SQLite dag_run rebuild doesn't cascade-delete task_instance rows.
  • Elasticsearch: remove the USE_PER_RUN_LOG_ID flag — defined but unused since ElasticsearchRemoteLogIO, so dead-code cleanup only.
  • OpenSearch: keep the hasattr(DagRun, "get_log_template") shim — per-run pinning must keep working on cores 3.0-3.3; on 3.4.0+ it self-disables and the handler falls back to [opensearch] log_id_template from config.
  • Make the shared create_log_template fixture version-adaptive so provider tests pass against all supported cores; update ES docs and add a significant-change newsfragment.

Behaviour change (core 3.4.0+): changing log_filename_template / log_id_template now applies immediately and retroactively to all DagRuns — there is no more per-run historical pinning, so logs written under a previous template are no longer reachable through the new one.


Was generative AI tooling used to co-author this PR?

@jason810496jason810496 self-assigned this Jul 7, 2026
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 04e801f to 81fa395CompareJuly 7, 2026 05:36
@jason810496jason810496 added the full tests needed We need to run full set of tests for this PR to merge label Jul 7, 2026
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 81fa395 to 1984f03CompareJuly 7, 2026 11:12
The ES/OpenSearch task handlers reached into the metadata DB directly
to read the per-DagRun log template, which conflicts with the Airflow 3
execution model where task handlers shouldn't query the DB directly.
Log filename/ID templates are now always read from live config instead
of being pinned to the value in effect when a DagRun was created.
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 1984f03 to 351b9adCompareJuly 9, 2026 05:20
@jason810496
jason810496 marked this pull request as ready for review July 9, 2026 05:22
@jason810496
jason810496 requested review from kaxil and potiukJuly 9, 2026 05:22
@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

The entire reason this exists is so that if the config changes the old logs are still viewable. This breaks that doesn't it

@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

a direct metadata-DB dependency that conflicts with the Airflow 3 execution model

This isn't true - we already handle this correctly since 3.0.0 by passing the log file suffix to the worker in ExecuteTaskWorkload; changes in 2.11 directly aren't available in 3.x either so that linked pr isn't really relevant

@jason810496

jason810496 commented Jul 9, 2026

Copy link
Copy Markdown
MemberAuthor

The entire reason this exists is so that if the config changes the old logs are still viewable. This breaks that doesn't it

Yes, but based on

  • a) Airflow 2.11.1 already disabled per-run template retrieval by default for security reasons (https://github.com/apache/airflow/pull/61880);
  • b) the ES / OS provider for Airflow 3 did respect the log_template (more details as below)

I thought we can introduce the breaking change in 3.4 to drop this feature.

a direct metadata-DB dependency that conflicts with the Airflow 3 execution model

This isn't true - we already handle this correctly since 3.0.0 by passing the log file suffix to the worker in ExecuteTaskWorkload; changes in 2.11 directly aren't available in 3.x either so that linked pr isn't really relevant

It's different part, not for the log file suffix, only the ES / OS consumed the LogTemplate DB model.

IIRC, the ES / OS provider (mainly #53821) that works with Airflow 3 didn't respect the log_template model (both write and read side will fetch the latest config value instead of going through the LogTemplate model. When accessing the log_template property on TI, it hit the prohibited commit exception, so we workaround as directly accessing the conf.) at all (I will double check for this part) this is the reason why I raise the discussion to drop the LogTemplate DB model.

Or another direction, I can make the OS and ES provider still respect the LogTemplate DB model (introducing LogTemplate accessor, or retrieve the LogTemplate at Execution API and set as the StartUpDetails, etc)
WDYT? Thanks.

@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

LogTemplate is used well beyond just ES -- it's used to generate the log file suffix that is passed to workers where they write the local log file, and for reading old task logs back from S3/GCS etc.

@jason810496

Copy link
Copy Markdown
MemberAuthor

LogTemplate is used well beyond just ES -- it's used to generate the log file suffix that is passed to workers where they write the local log file, and for reading old task logs back from S3/GCS etc.

Thanks for the clarification. I see, then I will go through the flow again then restoring the LogTemplate for ES should be the correct direction, thanks.

@jason810496

Copy link
Copy Markdown
MemberAuthor

Dropping the LogTemplate DB model is the wrong direction, create #69688 to resolve the root cause.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@jason810496@ashb
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Drop the LogTemplate DB model - #69520

Closed
jason810496 wants to merge 1 commit into
apache:mainfrom
jason810496:refactor/logging/drop-log-template-model
Closed

Drop the LogTemplate DB model#69520
jason810496 wants to merge 1 commit into
apache:mainfrom
jason810496:refactor/logging/drop-log-template-model

Conversation

@jason810496

@jason810496jason810496 commented Jul 7, 2026

Copy link
Copy Markdown
Member

Why

The LogTemplate model pinned historical values of [logging] log_filename_template / [elasticsearch] log_id_template per DagRun, and task handlers read it via dag_run.get_log_template(session=...) — a direct metadata-DB dependency that conflicts with the Airflow 3 execution model and couples log handlers to an Airflow-core model just to read two strings already available from config.

Airflow 2.11.1 already disabled per-run template retrieval by default for security reasons (#61880); this completes that direction on main by removing the model entirely.

What

  • Remove the LogTemplate model, the dag_run.log_template_id column/FK, DagRun.get_log_template(), and synchronize_log_template() with its call sites. FileTaskHandler._render_filename() now reads [logging] log_filename_template from live config at render time.
  • Add migration 0126_3_4_0_drop_log_template.py (downgrade recreates the column and table); both directions use disable_sqlite_fkeys(op) so the SQLite dag_run rebuild doesn't cascade-delete task_instance rows.
  • Elasticsearch: remove the USE_PER_RUN_LOG_ID flag — defined but unused since ElasticsearchRemoteLogIO, so dead-code cleanup only.
  • OpenSearch: keep the hasattr(DagRun, "get_log_template") shim — per-run pinning must keep working on cores 3.0-3.3; on 3.4.0+ it self-disables and the handler falls back to [opensearch] log_id_template from config.
  • Make the shared create_log_template fixture version-adaptive so provider tests pass against all supported cores; update ES docs and add a significant-change newsfragment.

Behaviour change (core 3.4.0+): changing log_filename_template / log_id_template now applies immediately and retroactively to all DagRuns — there is no more per-run historical pinning, so logs written under a previous template are no longer reachable through the new one.


Was generative AI tooling used to co-author this PR?

@jason810496jason810496 self-assigned this Jul 7, 2026
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 04e801f to 81fa395CompareJuly 7, 2026 05:36
@jason810496jason810496 added the full tests needed We need to run full set of tests for this PR to merge label Jul 7, 2026
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 81fa395 to 1984f03CompareJuly 7, 2026 11:12
The ES/OpenSearch task handlers reached into the metadata DB directly
to read the per-DagRun log template, which conflicts with the Airflow 3
execution model where task handlers shouldn't query the DB directly.
Log filename/ID templates are now always read from live config instead
of being pinned to the value in effect when a DagRun was created.
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 1984f03 to 351b9adCompareJuly 9, 2026 05:20
@jason810496
jason810496 marked this pull request as ready for review July 9, 2026 05:22
@jason810496
jason810496 requested review from kaxil and potiukJuly 9, 2026 05:22
@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

The entire reason this exists is so that if the config changes the old logs are still viewable. This breaks that doesn't it

@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

a direct metadata-DB dependency that conflicts with the Airflow 3 execution model

This isn't true - we already handle this correctly since 3.0.0 by passing the log file suffix to the worker in ExecuteTaskWorkload; changes in 2.11 directly aren't available in 3.x either so that linked pr isn't really relevant

@jason810496

jason810496 commented Jul 9, 2026

Copy link
Copy Markdown
MemberAuthor

The entire reason this exists is so that if the config changes the old logs are still viewable. This breaks that doesn't it

Yes, but based on

  • a) Airflow 2.11.1 already disabled per-run template retrieval by default for security reasons (https://github.com/apache/airflow/pull/61880);
  • b) the ES / OS provider for Airflow 3 did respect the log_template (more details as below)

I thought we can introduce the breaking change in 3.4 to drop this feature.

a direct metadata-DB dependency that conflicts with the Airflow 3 execution model

This isn't true - we already handle this correctly since 3.0.0 by passing the log file suffix to the worker in ExecuteTaskWorkload; changes in 2.11 directly aren't available in 3.x either so that linked pr isn't really relevant

It's different part, not for the log file suffix, only the ES / OS consumed the LogTemplate DB model.

IIRC, the ES / OS provider (mainly #53821) that works with Airflow 3 didn't respect the log_template model (both write and read side will fetch the latest config value instead of going through the LogTemplate model. When accessing the log_template property on TI, it hit the prohibited commit exception, so we workaround as directly accessing the conf.) at all (I will double check for this part) this is the reason why I raise the discussion to drop the LogTemplate DB model.

Or another direction, I can make the OS and ES provider still respect the LogTemplate DB model (introducing LogTemplate accessor, or retrieve the LogTemplate at Execution API and set as the StartUpDetails, etc)
WDYT? Thanks.

@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

LogTemplate is used well beyond just ES -- it's used to generate the log file suffix that is passed to workers where they write the local log file, and for reading old task logs back from S3/GCS etc.

@jason810496

Copy link
Copy Markdown
MemberAuthor

LogTemplate is used well beyond just ES -- it's used to generate the log file suffix that is passed to workers where they write the local log file, and for reading old task logs back from S3/GCS etc.

Thanks for the clarification. I see, then I will go through the flow again then restoring the LogTemplate for ES should be the correct direction, thanks.

@jason810496

Copy link
Copy Markdown
MemberAuthor

Dropping the LogTemplate DB model is the wrong direction, create #69688 to resolve the root cause.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@jason810496@ashb
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Drop the LogTemplate DB model - #69520

Closed
jason810496 wants to merge 1 commit into
apache:mainfrom
jason810496:refactor/logging/drop-log-template-model
Closed

Drop the LogTemplate DB model#69520
jason810496 wants to merge 1 commit into
apache:mainfrom
jason810496:refactor/logging/drop-log-template-model

Conversation

@jason810496

@jason810496jason810496 commented Jul 7, 2026

Copy link
Copy Markdown
Member

Why

The LogTemplate model pinned historical values of [logging] log_filename_template / [elasticsearch] log_id_template per DagRun, and task handlers read it via dag_run.get_log_template(session=...) — a direct metadata-DB dependency that conflicts with the Airflow 3 execution model and couples log handlers to an Airflow-core model just to read two strings already available from config.

Airflow 2.11.1 already disabled per-run template retrieval by default for security reasons (#61880); this completes that direction on main by removing the model entirely.

What

  • Remove the LogTemplate model, the dag_run.log_template_id column/FK, DagRun.get_log_template(), and synchronize_log_template() with its call sites. FileTaskHandler._render_filename() now reads [logging] log_filename_template from live config at render time.
  • Add migration 0126_3_4_0_drop_log_template.py (downgrade recreates the column and table); both directions use disable_sqlite_fkeys(op) so the SQLite dag_run rebuild doesn't cascade-delete task_instance rows.
  • Elasticsearch: remove the USE_PER_RUN_LOG_ID flag — defined but unused since ElasticsearchRemoteLogIO, so dead-code cleanup only.
  • OpenSearch: keep the hasattr(DagRun, "get_log_template") shim — per-run pinning must keep working on cores 3.0-3.3; on 3.4.0+ it self-disables and the handler falls back to [opensearch] log_id_template from config.
  • Make the shared create_log_template fixture version-adaptive so provider tests pass against all supported cores; update ES docs and add a significant-change newsfragment.

Behaviour change (core 3.4.0+): changing log_filename_template / log_id_template now applies immediately and retroactively to all DagRuns — there is no more per-run historical pinning, so logs written under a previous template are no longer reachable through the new one.


Was generative AI tooling used to co-author this PR?

@jason810496jason810496 self-assigned this Jul 7, 2026
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 04e801f to 81fa395CompareJuly 7, 2026 05:36
@jason810496jason810496 added the full tests needed We need to run full set of tests for this PR to merge label Jul 7, 2026
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 81fa395 to 1984f03CompareJuly 7, 2026 11:12
The ES/OpenSearch task handlers reached into the metadata DB directly
to read the per-DagRun log template, which conflicts with the Airflow 3
execution model where task handlers shouldn't query the DB directly.
Log filename/ID templates are now always read from live config instead
of being pinned to the value in effect when a DagRun was created.
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 1984f03 to 351b9adCompareJuly 9, 2026 05:20
@jason810496
jason810496 marked this pull request as ready for review July 9, 2026 05:22
@jason810496
jason810496 requested review from kaxil and potiukJuly 9, 2026 05:22
@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

The entire reason this exists is so that if the config changes the old logs are still viewable. This breaks that doesn't it

@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

a direct metadata-DB dependency that conflicts with the Airflow 3 execution model

This isn't true - we already handle this correctly since 3.0.0 by passing the log file suffix to the worker in ExecuteTaskWorkload; changes in 2.11 directly aren't available in 3.x either so that linked pr isn't really relevant

@jason810496

jason810496 commented Jul 9, 2026

Copy link
Copy Markdown
MemberAuthor

The entire reason this exists is so that if the config changes the old logs are still viewable. This breaks that doesn't it

Yes, but based on

  • a) Airflow 2.11.1 already disabled per-run template retrieval by default for security reasons (https://github.com/apache/airflow/pull/61880);
  • b) the ES / OS provider for Airflow 3 did respect the log_template (more details as below)

I thought we can introduce the breaking change in 3.4 to drop this feature.

a direct metadata-DB dependency that conflicts with the Airflow 3 execution model

This isn't true - we already handle this correctly since 3.0.0 by passing the log file suffix to the worker in ExecuteTaskWorkload; changes in 2.11 directly aren't available in 3.x either so that linked pr isn't really relevant

It's different part, not for the log file suffix, only the ES / OS consumed the LogTemplate DB model.

IIRC, the ES / OS provider (mainly #53821) that works with Airflow 3 didn't respect the log_template model (both write and read side will fetch the latest config value instead of going through the LogTemplate model. When accessing the log_template property on TI, it hit the prohibited commit exception, so we workaround as directly accessing the conf.) at all (I will double check for this part) this is the reason why I raise the discussion to drop the LogTemplate DB model.

Or another direction, I can make the OS and ES provider still respect the LogTemplate DB model (introducing LogTemplate accessor, or retrieve the LogTemplate at Execution API and set as the StartUpDetails, etc)
WDYT? Thanks.

@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

LogTemplate is used well beyond just ES -- it's used to generate the log file suffix that is passed to workers where they write the local log file, and for reading old task logs back from S3/GCS etc.

@jason810496

Copy link
Copy Markdown
MemberAuthor

LogTemplate is used well beyond just ES -- it's used to generate the log file suffix that is passed to workers where they write the local log file, and for reading old task logs back from S3/GCS etc.

Thanks for the clarification. I see, then I will go through the flow again then restoring the LogTemplate for ES should be the correct direction, thanks.

@jason810496

Copy link
Copy Markdown
MemberAuthor

Dropping the LogTemplate DB model is the wrong direction, create #69688 to resolve the root cause.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@jason810496@ashb
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Drop the LogTemplate DB model - #69520

Closed
jason810496 wants to merge 1 commit into
apache:mainfrom
jason810496:refactor/logging/drop-log-template-model
Closed

Drop the LogTemplate DB model#69520
jason810496 wants to merge 1 commit into
apache:mainfrom
jason810496:refactor/logging/drop-log-template-model

Conversation

@jason810496

@jason810496jason810496 commented Jul 7, 2026

Copy link
Copy Markdown
Member

Why

The LogTemplate model pinned historical values of [logging] log_filename_template / [elasticsearch] log_id_template per DagRun, and task handlers read it via dag_run.get_log_template(session=...) — a direct metadata-DB dependency that conflicts with the Airflow 3 execution model and couples log handlers to an Airflow-core model just to read two strings already available from config.

Airflow 2.11.1 already disabled per-run template retrieval by default for security reasons (#61880); this completes that direction on main by removing the model entirely.

What

  • Remove the LogTemplate model, the dag_run.log_template_id column/FK, DagRun.get_log_template(), and synchronize_log_template() with its call sites. FileTaskHandler._render_filename() now reads [logging] log_filename_template from live config at render time.
  • Add migration 0126_3_4_0_drop_log_template.py (downgrade recreates the column and table); both directions use disable_sqlite_fkeys(op) so the SQLite dag_run rebuild doesn't cascade-delete task_instance rows.
  • Elasticsearch: remove the USE_PER_RUN_LOG_ID flag — defined but unused since ElasticsearchRemoteLogIO, so dead-code cleanup only.
  • OpenSearch: keep the hasattr(DagRun, "get_log_template") shim — per-run pinning must keep working on cores 3.0-3.3; on 3.4.0+ it self-disables and the handler falls back to [opensearch] log_id_template from config.
  • Make the shared create_log_template fixture version-adaptive so provider tests pass against all supported cores; update ES docs and add a significant-change newsfragment.

Behaviour change (core 3.4.0+): changing log_filename_template / log_id_template now applies immediately and retroactively to all DagRuns — there is no more per-run historical pinning, so logs written under a previous template are no longer reachable through the new one.


Was generative AI tooling used to co-author this PR?

@jason810496jason810496 self-assigned this Jul 7, 2026
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 04e801f to 81fa395CompareJuly 7, 2026 05:36
@jason810496jason810496 added the full tests needed We need to run full set of tests for this PR to merge label Jul 7, 2026
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 81fa395 to 1984f03CompareJuly 7, 2026 11:12
The ES/OpenSearch task handlers reached into the metadata DB directly
to read the per-DagRun log template, which conflicts with the Airflow 3
execution model where task handlers shouldn't query the DB directly.
Log filename/ID templates are now always read from live config instead
of being pinned to the value in effect when a DagRun was created.
@jason810496
jason810496force-pushed the refactor/logging/drop-log-template-model branch from 1984f03 to 351b9adCompareJuly 9, 2026 05:20
@jason810496
jason810496 marked this pull request as ready for review July 9, 2026 05:22
@jason810496
jason810496 requested review from kaxil and potiukJuly 9, 2026 05:22
@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

The entire reason this exists is so that if the config changes the old logs are still viewable. This breaks that doesn't it

@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

a direct metadata-DB dependency that conflicts with the Airflow 3 execution model

This isn't true - we already handle this correctly since 3.0.0 by passing the log file suffix to the worker in ExecuteTaskWorkload; changes in 2.11 directly aren't available in 3.x either so that linked pr isn't really relevant

@jason810496

jason810496 commented Jul 9, 2026

Copy link
Copy Markdown
MemberAuthor

The entire reason this exists is so that if the config changes the old logs are still viewable. This breaks that doesn't it

Yes, but based on

  • a) Airflow 2.11.1 already disabled per-run template retrieval by default for security reasons (https://github.com/apache/airflow/pull/61880);
  • b) the ES / OS provider for Airflow 3 did respect the log_template (more details as below)

I thought we can introduce the breaking change in 3.4 to drop this feature.

a direct metadata-DB dependency that conflicts with the Airflow 3 execution model

This isn't true - we already handle this correctly since 3.0.0 by passing the log file suffix to the worker in ExecuteTaskWorkload; changes in 2.11 directly aren't available in 3.x either so that linked pr isn't really relevant

It's different part, not for the log file suffix, only the ES / OS consumed the LogTemplate DB model.

IIRC, the ES / OS provider (mainly #53821) that works with Airflow 3 didn't respect the log_template model (both write and read side will fetch the latest config value instead of going through the LogTemplate model. When accessing the log_template property on TI, it hit the prohibited commit exception, so we workaround as directly accessing the conf.) at all (I will double check for this part) this is the reason why I raise the discussion to drop the LogTemplate DB model.

Or another direction, I can make the OS and ES provider still respect the LogTemplate DB model (introducing LogTemplate accessor, or retrieve the LogTemplate at Execution API and set as the StartUpDetails, etc)
WDYT? Thanks.

@ashb

ashb commented Jul 9, 2026

Copy link
Copy Markdown
Member

LogTemplate is used well beyond just ES -- it's used to generate the log file suffix that is passed to workers where they write the local log file, and for reading old task logs back from S3/GCS etc.

@jason810496

Copy link
Copy Markdown
MemberAuthor

LogTemplate is used well beyond just ES -- it's used to generate the log file suffix that is passed to workers where they write the local log file, and for reading old task logs back from S3/GCS etc.

Thanks for the clarification. I see, then I will go through the flow again then restoring the LogTemplate for ES should be the correct direction, thanks.

@jason810496

Copy link
Copy Markdown
MemberAuthor

Dropping the LogTemplate DB model is the wrong direction, create #69688 to resolve the root cause.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@jason810496@ashb