You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "tirkarthi (via GitHub)" <gi...@apache.org> on 2025/02/17 12:07:18 UTC
[PR] Add log grouping using summary and details tag. [airflow]
tirkarthi opened a new pull request, #46820:
URL: https://github.com/apache/airflow/pull/46820
We extensively use log grouping in Airflow 2. While looking at implementation of grouping through a prism plugin to support colors I found about `<details>` and `<summary>` tag which has good browser support supporting almost all browsers in last 5 years.
The implementation is to keep the group name under `<details>` and to have the group's log content under `<summary>` section which can be clicked to expand like Airflow 2. The PR involves looking for "::group::" in the log line to indicate the log group start marker and collect the subsequent lines till line with "::endgroup::" is reached and wrap them up in the `<details>` tag as a section. The code logic and styling around this can be improved but I felt this is a good enough to open a PR for discussion. This also makes the implementation simpler compared to Airflow 2.
Docs : https://developer.mozilla.org/en-US/docs/Web/HTML/Element/details
Notes to reviewer and self
1. It seems "::group::<name>" is the documented approach though code also supports "##group##<name>" . Do we need to support this?
2. I just used split string by "::group::" and capture the part after marker as name instead of regex to extract group name for simplicity but it can be made as a regex.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1966109434
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,47 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ // TODO: Add support for nested groups
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
+ if (line.includes("::group::") && !startGroup) {
+ startGroup = true;
+ groupName = line.split("::group::")[1] as string;
+ } else if (line.includes("::endgroup::")) {
+ startGroup = false;
+ groupLines.push(line);
+ const group = (
+ <details key={groupName}>
+ <summary data-testid={`summary-${groupName}`}>
+ <Text as="span" color="fg.info" cursor="pointer">
Review Comment:
Background color works!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on PR #46820:
URL: https://github.com/apache/airflow/pull/46820#issuecomment-2692609042
Ok, I rebased and tried an approach here which would be to let `renderStructuredLog` form the elements. There is a package `react-innertext` through which the text could be obtained. Then I just continue processing like the earlier approach on strings with value from `innerText` to get beginning and end of the group markers. I couldn't add background color differentiation since the `renderStructuredLog` already returns HTML Element and then wrapping it again in Text shows React warning.
Current screenshot
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on PR #46820:
URL: https://github.com/apache/airflow/pull/46820#issuecomment-2691825019
> @Lee-W Sorry, by normal log I meant the Airflow 2.x logs which were mostly just string.
I think we'll need to check the content like Brent suggested
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1966587619
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,47 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ // TODO: Add support for nested groups
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
+ if (line.includes("::group::") && !startGroup) {
+ startGroup = true;
+ groupName = line.split("::group::")[1] as string;
+ } else if (line.includes("::endgroup::")) {
+ startGroup = false;
+ groupLines.push(line);
+ const group = (
+ <details key={groupName}>
Review Comment:
To the comment below - in order to have the color across the full width, I assume this is needed
```suggestion
<details key={groupName} style={{ width: "100%" }}>
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on PR #46820:
URL: https://github.com/apache/airflow/pull/46820#issuecomment-2692618446
The default timeout for `waitFor` is 1000ms (1 second) which might be causing random failures. Increased it to 10s and it seems to be fixed.
https://testing-library.com/docs/dom-testing-library/api-async/#waitfor
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on PR #46820:
URL: https://github.com/apache/airflow/pull/46820#issuecomment-2667706827
> Airflow 2 also went over the logs line-by-line. Don't know if you checked the old implementation. Do you think this could be factored-in?
Thanks, I checked it. From what I understand in Airflow 2 manually created span tags appending to each other and used `dangerouslySetInnerHTML` to set the manually created html to div. This is simpler and doesn't need that. There should be a way to take nested groups through recursion but I cannot get the correct approach. I would be okay for nested implementation to be added in another PR to not block this PR.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun merged PR #46820:
URL: https://github.com/apache/airflow/pull/46820
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1966588034
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,47 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ // TODO: Add support for nested groups
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
+ if (line.includes("::group::") && !startGroup) {
+ startGroup = true;
+ groupName = line.split("::group::")[1] as string;
+ } else if (line.includes("::endgroup::")) {
+ startGroup = false;
+ groupLines.push(line);
Review Comment:
I assume if you don't push this then the `::endgroup::` is just skipped
```suggestion
```
...then it comes.out as:
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #46820:
URL: https://github.com/apache/airflow/pull/46820#issuecomment-2667060256
I'd for sure strongly nested grouping - but I would assume this can be added later as well. I think this is a very big shortcoming in Github and I was a big fan that Airflow had this in.
Airflow 2 also went over the logs line-by-line. Don't know if you checked the old implementation. Do you think this could be factored-in?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1966588034
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,47 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ // TODO: Add support for nested groups
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
+ if (line.includes("::group::") && !startGroup) {
+ startGroup = true;
+ groupName = line.split("::group::")[1] as string;
+ } else if (line.includes("::endgroup::")) {
+ startGroup = false;
+ groupLines.push(line);
Review Comment:
I assume if you don't push this then the `::endgroup::` is just skipped
```suggestion
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on PR #46820:
URL: https://github.com/apache/airflow/pull/46820#issuecomment-2688291497
Let's rebase.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on PR #46820:
URL: https://github.com/apache/airflow/pull/46820#issuecomment-2694640664
Although it looks like sometimes we just repeat the log group message
<img width="843" alt="Screenshot 2025-03-03 at 9 43 59 AM" src="https://github.com/user-attachments/assets/4d602478-ff74-4c72-a094-5be398f4f80d" />
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on PR #46820:
URL: https://github.com/apache/airflow/pull/46820#issuecomment-2688685833
> I rebased and with structlog rendering I guess I might need to start from scratch on the implementation :( `renderStructuredLog` handles `logMessage` which could be both `string` and `StructuredLogMessage` . Is there a way to see from response headers or content if the log response is a task-sdk log and a normal log to switch between implementations?
I was afraid that change was going to be hard to rebase against.
@Lee-W Do you know the answer to this question?
I think regardless we need to check the logMessage string or `logMessage.event` string if it includes ::group::
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1966588754
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,47 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ // TODO: Add support for nested groups
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
+ if (line.includes("::group::") && !startGroup) {
+ startGroup = true;
+ groupName = line.split("::group::")[1] as string;
Review Comment:
If you return here after starting a new group, the first line is not repeating the group header:
```suggestion
groupName = line.split("::group::")[1] as string;
return undefined;
```
...then it is like:
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on PR #46820:
URL: https://github.com/apache/airflow/pull/46820#issuecomment-2689720558
@Lee-W Sorry, by normal log I meant the Airflow 2.x logs which were mostly just string.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1966107577
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,47 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ // TODO: Add support for nested groups
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
+ if (line.includes("::group::") && !startGroup) {
+ startGroup = true;
+ groupName = line.split("::group::")[1] as string;
+ } else if (line.includes("::endgroup::")) {
+ startGroup = false;
+ groupLines.push(line);
+ const group = (
+ <details key={groupName}>
+ <summary data-testid={`summary-${groupName}`}>
+ <Text as="span" color="fg.info" cursor="pointer">
Review Comment:
I just made the text "^^^Log group end" in order to visually mark it. I think indention is not sooo cool because then the date/time as prefix shifts as well and breaks the columns in reading. I'd rather vote for any other visual indicator. How about 10% ligher background as colored highlight? Or a HR tag == <hr />?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1966107577
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,47 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ // TODO: Add support for nested groups
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
+ if (line.includes("::group::") && !startGroup) {
+ startGroup = true;
+ groupName = line.split("::group::")[1] as string;
+ } else if (line.includes("::endgroup::")) {
+ startGroup = false;
+ groupLines.push(line);
+ const group = (
+ <details key={groupName}>
+ <summary data-testid={`summary-${groupName}`}>
+ <Text as="span" color="fg.info" cursor="pointer">
Review Comment:
I just made the text "^^^Log group end" in order to visually mark it. I think indention is not sooo cool because then the date/time as prefix shifts as well and breaks the columns in reading. I'd rather vote for any other visual indicator. How about 10% ligher background as colored highlight? Or a <hr />?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1973820190
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,47 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ // TODO: Add support for nested groups
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
+ if (line.includes("::group::") && !startGroup) {
+ startGroup = true;
+ groupName = line.split("::group::")[1] as string;
+ } else if (line.includes("::endgroup::")) {
+ startGroup = false;
+ groupLines.push(line);
+ const group = (
+ <details key={groupName}>
+ <summary data-testid={`summary-${groupName}`}>
+ <Text as="span" color="fg.info" cursor="pointer">
+ {groupName}
+ </Text>
+ </summary>
+ {groupLines.map((text, groupIndex) => (
+ <Text key={groupIndex} py={1}>
Review Comment:
I think bg.emphasized or subtle is fine
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1966067471
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,47 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ // TODO: Add support for nested groups
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
+ if (line.includes("::group::") && !startGroup) {
+ startGroup = true;
+ groupName = line.split("::group::")[1] as string;
+ } else if (line.includes("::endgroup::")) {
+ startGroup = false;
+ groupLines.push(line);
+ const group = (
+ <details key={groupName}>
+ <summary data-testid={`summary-${groupName}`}>
+ <Text as="span" color="fg.info" cursor="pointer">
Review Comment:
The Airflow 2 UI just replaced "::endgroup::" with "Log group end" . Shall I do the same here?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1963673964
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,47 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ // TODO: Add support for nested groups
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
+ if (line.includes("::group::") && !startGroup) {
+ startGroup = true;
+ groupName = line.split("::group::")[1] as string;
+ } else if (line.includes("::endgroup::")) {
+ startGroup = false;
+ groupLines.push(line);
+ const group = (
+ <details key={groupName}>
+ <summary data-testid={`summary-${groupName}`}>
+ <Text as="span" color="fg.info" cursor="pointer">
Review Comment:
It would be better if we made this an inline `<Button>` instead. Then keyboard navigation can find it.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #46820:
URL: https://github.com/apache/airflow/pull/46820#issuecomment-2664129081
> It seems "::group::" is the documented approach though code also supports "##group##" . Do we need to support this?
The syntax with `::` is the Github style. The variant with `##` is the way how ADO (MS AzureDevOps Pipelines) handle log grouping. As I was contributing this initially I wanted to make it compatible to both. If this is major complexity we can re-discuss. Else I'd propose to keep it.
> I just used split string by "::group::" and capture the part after marker as name instead of regex to extract group name for simplicity but it can be made as a regex.
I think this is OK.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1958803871
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,46 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
Review Comment:
This is a logic block which would be perfect to a unit test. Could you extract this into a functions and add a test case? (I know we have a very low coverage but this is an ideal candidate)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1966679832
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,47 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ // TODO: Add support for nested groups
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
+ if (line.includes("::group::") && !startGroup) {
+ startGroup = true;
+ groupName = line.split("::group::")[1] as string;
+ } else if (line.includes("::endgroup::")) {
+ startGroup = false;
+ groupLines.push(line);
+ const group = (
+ <details key={groupName}>
+ <summary data-testid={`summary-${groupName}`}>
+ <Text as="span" color="fg.info" cursor="pointer">
+ {groupName}
+ </Text>
+ </summary>
+ {groupLines.map((text, groupIndex) => (
+ <Text key={groupIndex} py={1}>
Review Comment:
I am not sure about the accessibility aspects of a lighter background for the text. I will leave this to @bbovenzi and would be happy to add if it's accessible.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1966441023
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,47 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ // TODO: Add support for nested groups
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
+ if (line.includes("::group::") && !startGroup) {
+ startGroup = true;
+ groupName = line.split("::group::")[1] as string;
+ } else if (line.includes("::endgroup::")) {
+ startGroup = false;
+ groupLines.push(line);
+ const group = (
+ <details key={groupName}>
+ <summary data-testid={`summary-${groupName}`}>
+ <Text as="span" color="fg.info" cursor="pointer">
Review Comment:
Adding `backgroundColor="bg.muted" color="fg.info"` to `::endgroup::` line gives below effect. Please let me know if you have any other suggestions.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on PR #46820:
URL: https://github.com/apache/airflow/pull/46820#issuecomment-2688607566
I rebased and with structlog rendering I guess I might need to start from scratch on the implementation :( `renderStructuredLog` handles `logMessage` which could be both `string` and `StructuredLogMessage` . Is there a way to see from response headers or content if the log response is a task-sdk log and a normal log to switch between implementations?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on PR #46820:
URL: https://github.com/apache/airflow/pull/46820#issuecomment-2694535540
I think the screenshot you have there looks good.
Nice catch on the timeout.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1973817509
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,47 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ // TODO: Add support for nested groups
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
+ if (line.includes("::group::") && !startGroup) {
+ startGroup = true;
+ groupName = line.split("::group::")[1] as string;
+ } else if (line.includes("::endgroup::")) {
+ startGroup = false;
+ groupLines.push(line);
+ const group = (
+ <details key={groupName}>
+ <summary data-testid={`summary-${groupName}`}>
+ <Text as="span" color="fg.info" cursor="pointer">
+ {groupName}
+ </Text>
+ </summary>
+ {groupLines.map((text, groupIndex) => (
+ <Text key={groupIndex} py={1}>
Review Comment:
A lighter background is fine for a11y
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,47 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ // TODO: Add support for nested groups
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
+ if (line.includes("::group::") && !startGroup) {
+ startGroup = true;
+ groupName = line.split("::group::")[1] as string;
+ } else if (line.includes("::endgroup::")) {
+ startGroup = false;
+ groupLines.push(line);
+ const group = (
+ <details key={groupName}>
+ <summary data-testid={`summary-${groupName}`}>
+ <Text as="span" color="fg.info" cursor="pointer">
+ {groupName}
+ </Text>
+ </summary>
+ {groupLines.map((text, groupIndex) => (
+ <Text key={groupIndex} py={1}>
Review Comment:
A lighter background is fine for a11y
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1966084301
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,47 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ // TODO: Add support for nested groups
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
+ if (line.includes("::group::") && !startGroup) {
+ startGroup = true;
+ groupName = line.split("::group::")[1] as string;
+ } else if (line.includes("::endgroup::")) {
+ startGroup = false;
+ groupLines.push(line);
+ const group = (
+ <details key={groupName}>
+ <summary data-testid={`summary-${groupName}`}>
+ <Text as="span" color="fg.info" cursor="pointer">
Review Comment:
We could also make it obvious indenting everything inside the group like we do for task groups in the grid view. Then we don't need any end indicator
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "Lee-W (via GitHub)" <gi...@apache.org>.
Lee-W commented on PR #46820:
URL: https://github.com/apache/airflow/pull/46820#issuecomment-2689716593
> Is there a way to see from response headers or content if the log response is a task-sdk log and a normal log to switch between implementations?
In the latest change, we make `StructuredLogMessage` starts like this
```python
StructuredLogMessage(event="::group::Log message source details", sources=source_list), # type: ignore[call-arg]
StructuredLogMessage(event="::endgroup::"),
```
This logic is implemented in `airflow/utils/log/file_task_handler.py` so I'm not sure what do you mean by task-sdk and normal ones 🤔
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on PR #46820:
URL: https://github.com/apache/airflow/pull/46820#issuecomment-2666205423
I think we need to make sure this is compatible with the log handler changes here: https://github.com/apache/airflow/pull/46827
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1959861211
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,46 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
Review Comment:
Thanks, added tests along with testing click to expand of the section,
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1966587820
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,47 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ // TODO: Add support for nested groups
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
+ if (line.includes("::group::") && !startGroup) {
+ startGroup = true;
+ groupName = line.split("::group::")[1] as string;
+ } else if (line.includes("::endgroup::")) {
+ startGroup = false;
+ groupLines.push(line);
+ const group = (
+ <details key={groupName}>
+ <summary data-testid={`summary-${groupName}`}>
+ <Text as="span" color="fg.info" cursor="pointer">
+ {groupName}
+ </Text>
+ </summary>
+ {groupLines.map((text, groupIndex) => (
+ <Text key={groupIndex} py={1}>
Review Comment:
This background adds some highlighting, `bg.muted` is the same that the background already has:
```suggestion
<Text backgroundColor="bg.emphasized" key={groupIndex} py={1}>
```
It would then come out like:
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1966030779
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,47 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ // TODO: Add support for nested groups
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
+ if (line.includes("::group::") && !startGroup) {
+ startGroup = true;
+ groupName = line.split("::group::")[1] as string;
+ } else if (line.includes("::endgroup::")) {
+ startGroup = false;
+ groupLines.push(line);
+ const group = (
+ <details key={groupName}>
+ <summary data-testid={`summary-${groupName}`}>
+ <Text as="span" color="fg.info" cursor="pointer">
Review Comment:
<img width="992" alt="Screenshot 2025-02-21 at 2 01 09 PM" src="https://github.com/user-attachments/assets/07924e88-f4c5-4199-98b8-9f7ad60235a5" />
You're right. It did detect it.
But let's replace `::end_group::``?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1966586525
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,47 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ // TODO: Add support for nested groups
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
+ if (line.includes("::group::") && !startGroup) {
+ startGroup = true;
+ groupName = line.split("::group::")[1] as string;
+ } else if (line.includes("::endgroup::")) {
+ startGroup = false;
+ groupLines.push(line);
+ const group = (
+ <details key={groupName}>
+ <summary data-testid={`summary-${groupName}`}>
+ <Text as="span" color="fg.info" cursor="pointer">
Review Comment:
If I take a look, just started locally - I don't see a visual difference between the log block in general and a color change.
Once the bg color is changed, also I'd recommend to drop the `::endgroup::` line from the log display, this is just used as marker.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on PR #46820:
URL: https://github.com/apache/airflow/pull/46820#issuecomment-2692614469
Strangely `pnpm test` fails but `pnpm test src/pages/TaskInstance/Logs/Logs.test.tsx` passes locally. Is there something I am missing while running tests in parallel?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on PR #46820:
URL: https://github.com/apache/airflow/pull/46820#issuecomment-2694766485
Task sdk logs render the event which has group name along with any other key=value pair as single string so while parsing this after the concatenation looks like the actual log group name. The concatenation and grouping are done in different functions. Not sure how to fix it.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on PR #46820:
URL: https://github.com/apache/airflow/pull/46820#issuecomment-2665894739
Thanks @jscheffl for the review. Glad to have the points since you implemented it in Airflow 2.
> The syntax with :: is the Github style. The variant with ## is the way how ADO (MS AzureDevOps Pipelines) handle log grouping. As I was contributing this initially I wanted to make it compatible to both. If this is major complexity we can re-discuss. Else I'd propose to keep it.
Supporting Azure would involve using regex and another branch along with handling of mix match between github and azure which will make this implementation complex.
> Before I manually test, does it support nested grouping? (Github does not and I was always proud Airflow does. Code looks like but have not checked.
Nested groups are unfortunately not supported at the moment since I go through each line and then add it to an array to empty it once the group ends. Nested groups might take something like a stack of lines to push and pop along with embedding the summary and details tag correctly inside. I have added a todo about the same.
I used the below dag to generate logs of various sizes from 1 to 80 MB and I don't see unexpected delay other than few seconds of delay to render the larger log files in the UI along with downloading. Speaking of this there used to be a limit for larger logs in old UI with a download button to view it. Download button was also handy tool for offline views and analysis which is missing in the new UI currently.
I guess this is good enough for an initial implementation and can always be iterated through later since we use this feature very much internally to handle logs.
```python
from datetime import datetime
import uuid
from airflow import DAG
from airflow.decorators import task
with DAG(
dag_id="log_grouping",
start_date=datetime(2025, 2, 1),
catchup=False,
schedule=None,
) as dag:
@task
def generate(**context):
line_limit = int(context["params"]["line_limit"])
group_limit = int(context["params"]["group_limit"])
print("::group::group name")
for index in range(group_limit):
print(f"::group::inner group name {index}")
for index in range(line_limit):
print(" ".join([uuid.uuid4().hex] * 1))
print("::endgroup::")
print("::endgroup::")
generate()
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on PR #46820:
URL: https://github.com/apache/airflow/pull/46820#issuecomment-2666255177
Thanks, whichever gets merged first I can try to fix the compatibility issues. I guess color highlighting based on keywords is also easier since I just need to add `color` attribute to `Text` based on keyword present in the line which can be done in a follow up PR before release.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add log grouping using summary and details tag. [airflow]
Posted by "tirkarthi (via GitHub)" <gi...@apache.org>.
tirkarthi commented on code in PR #46820:
URL: https://github.com/apache/airflow/pull/46820#discussion_r1963798724
##########
airflow/ui/src/queries/useLogs.tsx:
##########
@@ -49,8 +50,47 @@ const parseLogs = ({ data }: ParseLogsProps) => {
return { data, warning };
}
- // eslint-disable-next-line react/no-array-index-key
- const parsedLines = lines.map((line, index) => <p key={index}>{line}</p>);
+ let startGroup = false;
+ let groupLines: Array<string> = [];
+ let groupName = "";
+
+ // TODO: Add support for nested groups
+ /* eslint-disable react/no-array-index-key */
+ const parsedLines = lines.map((line, index) => {
+ if (line.includes("::group::") && !startGroup) {
+ startGroup = true;
+ groupName = line.split("::group::")[1] as string;
+ } else if (line.includes("::endgroup::")) {
+ startGroup = false;
+ groupLines.push(line);
+ const group = (
+ <details key={groupName}>
+ <summary data-testid={`summary-${groupName}`}>
+ <Text as="span" color="fg.info" cursor="pointer">
Review Comment:
I am able to navigate through links using tab/shift+tab and press enter to expand the section on Firefox 135 in Ubuntu 20.04 . Am I missing something here?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org