pierrejeambrun commented on code in PR #63467:
URL: https://github.com/apache/airflow/pull/63467#discussion_r3073614526
##########
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:
Was the structural change replacing the old <details>/<summary> HTML-based
group collapsing with a flat entry model + useLogGroups hook + virtual scroll
integration necessary in this PR?
It looks unrelated and I would have prefered we had this split into a
different PR. It's making the review process more complicated but since we are
far in the review. Lets continue like this and keep the change here.
(And in case of regression this will be harder to track down)
--
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]
