kaxil commented on code in PR #67207:
URL: https://github.com/apache/airflow/pull/67207#discussion_r3307234176
##########
task-sdk/src/airflow/sdk/definitions/decorators/task_group.py:
##########
@@ -94,8 +94,11 @@ def __call__(self, *args: FParams.args, **kwargs:
FParams.kwargs) -> DAGNode:
def _create_task_group(self, tg_factory: Callable[..., TaskGroup], *args:
Any, **kwargs: Any) -> DAGNode:
with tg_factory(add_suffix_on_collision=True, **self.tg_kwargs) as
task_group:
- if self.function.__doc__ and not task_group.tooltip:
- task_group.tooltip = self.function.__doc__
+ if self.function.__doc__:
+ if not task_group.tooltip:
+ task_group.tooltip = self.function.__doc__
+ if not task_group.doc_md:
+ task_group.doc_md = inspect.cleandoc(self.function.__doc__)
Review Comment:
Same docstring, two normalizations: `tooltip` gets the raw `__doc__`,
`doc_md` gets `inspect.cleandoc(__doc__)`. Either both should cleandoc or
neither should (and the cleandoc happens at the API response layer, matching
how DAG `doc_md` works today). Worth picking one rule and applying it
consistently:
```python
if doc := self.function.__doc__:
cleaned = inspect.cleandoc(doc)
if not task_group.tooltip:
task_group.tooltip = cleaned
if not task_group.doc_md:
task_group.doc_md = cleaned
```
Also, `task_group.doc_md = ...` runs the attrs converter on setattr. If the
cleandoc'd docstring happens to end with `.md` (e.g., last line is "see
README.md"), `_convert_doc_md` will try to open it as a file. Inherited from
DAG, but worth knowing.
##########
airflow-core/src/airflow/api_fastapi/core_api/datamodels/ui/common.py:
##########
@@ -67,6 +67,7 @@ class GridNodeResponse(BaseModel):
children: list[GridNodeResponse] | None = None
is_mapped: bool | None
setup_teardown_type: Literal["setup", "teardown"] | None = None
+ doc_md: str | None = None
Review Comment:
The DAG `doc_md` response runs `inspect.cleandoc` via a `field_validator`
(see `datamodels/dags.py:218-224`). This one doesn't, so a literal
`TaskGroup(doc_md=""" \n # Heading\n - item\n """)` ships to the UI with
its 4-space indent intact, and ReactMarkdown renders it as an indented code
block instead of a heading + list. The `@task_group` decorator path runs
cleandoc manually, but anyone using the class constructor directly hits this
footgun.
Adding a `@field_validator("doc_md", mode="before")` here, mirroring the DAG
one, would handle both cases at the same layer and let you drop the cleandoc
call from the decorator.
##########
airflow-core/src/airflow/api_fastapi/core_api/services/ui/task_group.py:
##########
@@ -110,9 +112,13 @@ def task_group_to_dict_grid(task_item_or_group,
parent_group_is_mapped=False):
for x in task_group_sort(task_group)
]
- return {
+ node = {
"id": task_group.group_id,
"label": task_group.group_display_name or task_group.label,
"is_mapped": mapped or None,
"children": children or None,
}
+ doc_md = getattr(task_group, "doc_md", None)
Review Comment:
`getattr(task_group, "doc_md", None)` with a default isn't needed --
`SerializedTaskGroup` declares `doc_md` as an attrs field with `default=None`,
so the attribute always exists on every code path that reaches here. Direct
access (`task_group.doc_md`) is what the rest of this function does for
`tooltip`, `group_id`, etc. If access ever fails, an `AttributeError` is a real
bug worth surfacing rather than papering over.
##########
airflow-core/src/airflow/ui/public/i18n/locales/en/common.json:
##########
@@ -270,6 +270,7 @@
"taskGroup": "Task Group",
"taskGroup_one": "Task Group",
"taskGroup_other": "Task Groups",
+ "taskGroupDocumentation": "Task Group Documentation",
Review Comment:
Two small things:
1. Existing pattern is namespaced (`task.documentation`,
`dagDetails.documentation`). This adds a top-level `taskGroupDocumentation`. A
`taskGroup.documentation` nested key would match the existing convention.
2. Only `en/common.json` is updated. The other locale dirs (`de`, `es`,
`fr`, `ko`, ...) keep the English fallback working, but the convention here is
to seed the English string into every locale's `common.json` so missing-key
fallback is consistent (see how #67540 handled the recent ko addition).
--
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]
