antonio-mello-ai commented on code in PR #63467:
URL: https://github.com/apache/airflow/pull/63467#discussion_r3073790412
##########
airflow-core/src/airflow/ui/src/queries/useLogs.tsx:
##########
@@ -108,75 +108,56 @@ const parseLogs = ({
return { data, warning };
}
- parsedLines = (() => {
- type Group = { level: number; lines: Array<JSX.Element | "">; name: string
};
+ const flatEntries: Array<ParsedLogEntry> = (() => {
+ type Group = { id: number; level: number; name: string };
const groupStack: Array<Group> = [];
- const result: Array<JSX.Element | ""> = [];
+ const result: Array<ParsedLogEntry> = [];
+ let nextGroupId = 0;
parsedLines.forEach((line) => {
const text = innerText(line);
if (text.includes("::group::")) {
const groupName = text.split("::group::")[1] as string;
+ const id = nextGroupId;
- groupStack.push({ level: groupStack.length, lines: [], name: groupName
});
+ nextGroupId += 1;
+ const level = groupStack.length;
+ const parentGroup = groupStack[groupStack.length - 1];
+
+ groupStack.push({ id, level, name: groupName });
+ result.push({
+ element: groupName,
+ group: { id, level, parentId: parentGroup?.id, type: "header" },
+ });
return;
}
if (text.includes("::endgroup::")) {
- const finishedGroup = groupStack.pop();
-
- if (finishedGroup) {
- const groupElement = (
- <Box key={finishedGroup.name} mb={2} pl={finishedGroup.level * 2}>
- <chakra.details open={open} w="100%">
- <chakra.summary data-testid={`summary-${finishedGroup.name}`}>
- <chakra.span color="fg.info" cursor="pointer">
- {finishedGroup.name}
- </chakra.span>
- </chakra.summary>
- {finishedGroup.lines}
- </chakra.details>
- </Box>
- );
Review Comment:
Fair point — happy to explain the reasoning.
The `<details>/<summary>` model collapsed each `::group::...::endgroup::`
block into a single entry in `parsedLogs`, so search only ever saw one
matchable "line" per group (the summary). This was the root cause of the bug
@bbovenzi flagged on Apr 8: *"The entire log group is being treated as a single
log line, so any searching inside a log group isn't useful."* That wasn't
fixable without restructuring the parse output.
The flat-entry model keeps each line inside a group as its own entry with
`groupId` metadata, and `useLogGroups` manages the expand/collapse + index
mapping. Search then iterates lines normally, and navigating to a match inside
a collapsed group auto-expands it.
Virtualization came along because the flat model multiplied row counts in
large logs — without it, a big expanded group would hurt render time on grouped
logs (the original `<details>` behavior deferred DOM work to browser-native
collapse).
Agreed on the PR hygiene point — in hindsight two separate PRs would have
made this easier to review, and I'll split future work more aggressively.
--
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]
