-
Notifications
You must be signed in to change notification settings - Fork 16.8k
Fix small max templated field length #59999
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ddc9e04
eef42b2
e1ea0fc
b1681c8
10a7af4
264c7b5
945f824
0ceda82
5ea5810
cac94f6
41a7f3c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -119,10 +119,10 @@ path = "src/airflow/sdk/__init__.py" | |
| "../shared/dagnode/src/airflow_shared/dagnode" = "src/airflow/sdk/_shared/dagnode" | ||
| "../shared/logging/src/airflow_shared/logging" = "src/airflow/sdk/_shared/logging" | ||
| "../shared/module_loading/src/airflow_shared/module_loading" = "src/airflow/sdk/_shared/module_loading" | ||
| "../shared/observability/src/airflow_shared/observability" = "src/airflow/_shared/observability" | ||
| "../shared/secrets_backend/src/airflow_shared/secrets_backend" = "src/airflow/sdk/_shared/secrets_backend" | ||
| "../shared/secrets_masker/src/airflow_shared/secrets_masker" = "src/airflow/sdk/_shared/secrets_masker" | ||
| "../shared/timezones/src/airflow_shared/timezones" = "src/airflow/sdk/_shared/timezones" | ||
| "../shared/observability/src/airflow_shared/observability" = "src/airflow/sdk/_shared/observability" | ||
| "../shared/secrets_backend/src/airflow_shared/secrets_backend" = "src/airflow/sdk/_shared/secrets_backend" | ||
| "../shared/secrets_masker/src/airflow_shared/secrets_masker" = "src/airflow/sdk/_shared/secrets_masker" | ||
| "../shared/timezones/src/airflow_shared/timezones" = "src/airflow/sdk/_shared/timezones" | ||
|
Comment on lines
-122
to
+125
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you do not implement a shared module then you should revert this. |
||
|
|
||
| [tool.hatch.build.targets.wheel] | ||
| packages = ["src/airflow"] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,12 +25,12 @@ | |
| import os | ||
| import sys | ||
| import time | ||
| from collections.abc import Callable, Iterable, Iterator, Mapping | ||
| from collections.abc import Callable, Iterable, Iterator, Mapping, Sequence | ||
| from contextlib import suppress | ||
| from datetime import datetime, timezone | ||
| from itertools import product | ||
| from pathlib import Path | ||
| from typing import TYPE_CHECKING, Annotated, Any, Literal | ||
| from typing import TYPE_CHECKING, Annotated, Any, Literal, Union | ||
| from urllib.parse import quote | ||
|
|
||
| import attrs | ||
|
|
@@ -104,6 +104,7 @@ | |
| TriggerDagRun, | ||
| ValidateInletsAndOutlets, | ||
| ) | ||
|
|
||
| from airflow.sdk.execution_time.context import ( | ||
| ConnectionAccessor, | ||
| InletEventsAccessors, | ||
|
|
@@ -136,6 +137,35 @@ class TaskRunnerMarker: | |
| """Marker for listener hooks, to properly detect from which component they are called.""" | ||
|
|
||
|
|
||
| def truncate_rendered_value(rendered: Any, max_length: int) -> str: | ||
| """ | ||
| Truncate rendered value with a reasonable minimum length to avoid edge cases. | ||
|
|
||
| Args: | ||
| rendered: The rendered value to truncate | ||
| max_length: The maximum allowed length for the output | ||
|
|
||
| Returns: | ||
| Truncated string that respects the max_length constraint | ||
| """ | ||
| # Set a reasonable minimum to avoid complex edge cases with very small values | ||
| if max_length < 100: | ||
| max_length = 100 | ||
|
|
||
| prefix = "Truncated. You can change this behaviour in [core]max_templated_field_length. " | ||
| suffix = "... " | ||
|
|
||
| # Apply repr() to the content first to account for quotes that will be added | ||
| content_repr = repr(rendered) | ||
| available_length = max_length - len(prefix) - len(suffix) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done sir |
||
| if available_length <= 0: | ||
| return (prefix + suffix)[:max_length] | ||
|
|
||
| content_part = content_repr[:available_length] | ||
| return f"{prefix}{content_part}{suffix}" | ||
|
|
||
|
|
||
|
|
||
| # TODO: Move this entire class into a separate file: | ||
| # `airflow/sdk/execution_time/task_instance.py` | ||
| # or `airflow/sdk/execution_time/runtime_ti.py` | ||
|
|
@@ -877,10 +907,7 @@ def sort_dict_recursively(obj: Any) -> Any: | |
| serialized = str(template_field) | ||
| if len(serialized) > max_length: | ||
| rendered = redact(serialized, name) | ||
| return ( | ||
| "Truncated. You can change this behaviour in [core]max_templated_field_length. " | ||
| f"{rendered[: max_length - 79]!r}... " | ||
| ) | ||
| return truncate_rendered_value(rendered, max_length) | ||
| return serialized | ||
| if not template_field and not isinstance(template_field, tuple): | ||
| # Avoid unnecessary serialization steps for empty fields unless they are tuples | ||
|
|
@@ -894,10 +921,7 @@ def sort_dict_recursively(obj: Any) -> Any: | |
| serialized = str(template_field) | ||
| if len(serialized) > max_length: | ||
| rendered = redact(serialized, name) | ||
| return ( | ||
| "Truncated. You can change this behaviour in [core]max_templated_field_length. " | ||
| f"{rendered[: max_length - 79]!r}... " | ||
| ) | ||
| return truncate_rendered_value(rendered, max_length) | ||
| return template_field | ||
|
|
||
|
|
||
|
|
@@ -1683,4 +1707,4 @@ def reinit_supervisor_comms() -> None: | |
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() | ||
| main() | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you do not implement a shared module then you should revert this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done sir , i have reverted the changes