pierrejeambrun commented on code in PR #61878:
URL: https://github.com/apache/airflow/pull/61878#discussion_r2841077746
##########
airflow-core/src/airflow/utils/helpers.py:
##########
@@ -50,6 +50,55 @@
S = TypeVar("S")
+def _truncate_rendered_value(rendered: str, max_length: int) -> str:
+ MIN_CONTENT_LENGTH = 7
+
+ if max_length <= 0:
+ return ""
+
+ # Build truncation message once, return if max_length is too small
+ prefix = "Truncated. You can change this behaviour in
[core]max_templated_field_length. "
+ suffix = "..."
+ trunc_only = f"{prefix}{suffix}"
+
+ if max_length < len(trunc_only):
+ return trunc_only
+
+ # Determine quoting strategy, compute overhead, calculate available space
+ value = rendered
+
+ # Determine quoting strategy: preserve existing quotes or choose
appropriate ones
+ has_outer_quotes = (value.startswith('"') and value.endswith('"')) or (
+ value.startswith("'") and value.endswith("'")
+ )
+
+ if has_outer_quotes:
+ quote_char = value[0]
+ content = value[1:-1]
+ else:
+ quote_char = '"' if "'" in value and '"' not in value else "'"
+ content = value
+
+ # Compute formatting overhead and calculate available space
+ overhead = len(prefix) + 2 + len(suffix) # prefix + opening quote +
closing quote + suffix
+ available = max_length - overhead
+
+ if available < MIN_CONTENT_LENGTH:
+ return trunc_only
+
+ # Slice content to fit, construct final string, ensure it doesn't exceed
max_length
+ content = content[:available].rstrip()
+ result = f"{prefix}{quote_char}{content}{quote_char}{suffix}"
+
+ # Ensure result is strictly less than max_length (trim if necessary)
+ if len(result) >= max_length:
+ excess = len(result) - max_length + 1
+ content = content[: len(content) - excess].rstrip()
+ result = f"{prefix}{quote_char}{content}{quote_char}{suffix}"
+
+ return result
Review Comment:
If we remove the quoting strategy and that the above logic is correct, that
shouldn't be needed. (To retruncate again before returning). This shouldn't
happen (maybe log a warning of something instead)
##########
airflow-core/src/airflow/utils/helpers.py:
##########
@@ -50,6 +50,55 @@
S = TypeVar("S")
+def _truncate_rendered_value(rendered: str, max_length: int) -> str:
+ MIN_CONTENT_LENGTH = 7
+
+ if max_length <= 0:
+ return ""
+
+ # Build truncation message once, return if max_length is too small
+ prefix = "Truncated. You can change this behaviour in
[core]max_templated_field_length. "
+ suffix = "..."
+ trunc_only = f"{prefix}{suffix}"
+
+ if max_length < len(trunc_only):
+ return trunc_only
+
+ # Determine quoting strategy, compute overhead, calculate available space
+ value = rendered
+
+ # Determine quoting strategy: preserve existing quotes or choose
appropriate ones
+ has_outer_quotes = (value.startswith('"') and value.endswith('"')) or (
+ value.startswith("'") and value.endswith("'")
+ )
+
+ if has_outer_quotes:
+ quote_char = value[0]
+ content = value[1:-1]
+ else:
+ quote_char = '"' if "'" in value and '"' not in value else "'"
+ content = value
+
+ # Compute formatting overhead and calculate available space
+ overhead = len(prefix) + 2 + len(suffix) # prefix + opening quote +
closing quote + suffix
+ available = max_length - overhead
Review Comment:
Not sure it's worth to go through the trouble of quotings.
##########
airflow-core/src/airflow/utils/helpers.py:
##########
@@ -50,6 +50,55 @@
S = TypeVar("S")
+def _truncate_rendered_value(rendered: str, max_length: int) -> str:
Review Comment:
Why is the function declared 'private' if it's meant to be imported in
task_runner and serialization module?
Not sure the linter will like that.
--
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]
