Update TaskLogContent to support virtualized rendering
#50746
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
guan404ming
requested review from
bbovenzi,
jscheffl,
pierrejeambrun,
ryanahamilton and
shubhamraj-git
as code owners
Contributor
|
Cool! I feel not confident to review but very great improvement! |
guan404ming
force-pushed
the
refactor-ti-log
branch
from
e59c858 to
f8233fb
Compare
Member
Author
|
Thanks! I'm still trying to figure out how to test virtualized list. |
Collaborator
|
Nice change! Good job! |
guan404ming
force-pushed
the
refactor-ti-log
branch
from
f8233fb to
2ea9bb8
Compare
guan404ming
commented
May 18, 2025
airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/Logs.test.tsx
Outdated
Show resolved
Hide resolved
guan404ming
force-pushed
the
refactor-ti-log
branch
from
2ea9bb8 to
adfb2f0
Compare
guan404ming
force-pushed
the
refactor-ti-log
branch
4 times, most recently
from
3e1bc49 to
2a9eb7b
Compare
bbovenzi
reviewed
May 22, 2025
airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/TaskLogContent.tsx
Show resolved
Hide resolved
bbovenzi
reviewed
May 22, 2025
bbovenzi
reviewed
May 22, 2025
airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/Logs.tsx
Outdated
Show resolved
Hide resolved
guan404ming
force-pushed
the
refactor-ti-log
branch
from
2a9eb7b to
e390fb4
Compare
Member
Author
|
Currently the list is worked in large monitor as in the small screen Here are screenshots on 27 inch monitor |
guan404ming
force-pushed
the
refactor-ti-log
branch
from
e390fb4 to
c9c0cfb
Compare
guan404ming
force-pushed
the
refactor-ti-log
branch
from
c9c0cfb to
2f98bea
Compare
bbovenzi
reviewed
May 28, 2025
airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/TaskLogContent.tsx
Outdated
Show resolved
Hide resolved
Contributor
|
Looks good overall. Just one nit. And then, I wonder if we want this in 3.0.2 instead of 3.1.0? Its an enhancement, not a new feature. |
Member
Author
I think it should go into 3.0.2 since it doesn’t affect how users interact with the component. I’d consider it more of an enhancement for ux. Imo the new feature would be the upcoming ndjson-related changes after #49470 |
bbovenzi
added
the
backport-to-v3-1-test
bbovenzi
approved these changes
May 29, 2025
Backport successfully created: v3-0-test
|
Member
Author
|
Thanks! |
Member
|
Big thanks @guan404ming 🙌 That's a fantastic improvement, really appreciate your work on this! |
kaxil
pushed a commit
that referenced
this pull request
Jun 3, 2025
…50746) (#51202) * Fix OpenAPI schema for `get_log` API (#50547) * Fix openapi schema for get_log API * Fix test_log (cherry picked from commit 08cc57d) * [v3-0-test] Update `TaskLogContent` to support virtualized rendering (#50746) * Update TaskLogContent to support virtualized rendering * Update TaskLogPreview and Logs to handle undefined parsedLogs (cherry picked from commit 813f3e3) Co-authored-by: Guan Ming(Wesley) Chiu <105915352+guan404ming@users.noreply.github.com> --------- Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com> Co-authored-by: Guan Ming(Wesley) Chiu <105915352+guan404ming@users.noreply.github.com>
kaxil
pushed a commit
that referenced
this pull request
Jun 3, 2025
…50746) (#51202) * Fix OpenAPI schema for `get_log` API (#50547) * Fix openapi schema for get_log API * Fix test_log (cherry picked from commit 08cc57d) * [v3-0-test] Update `TaskLogContent` to support virtualized rendering (#50746) * Update TaskLogContent to support virtualized rendering * Update TaskLogPreview and Logs to handle undefined parsedLogs (cherry picked from commit 813f3e3) Co-authored-by: Guan Ming(Wesley) Chiu <105915352+guan404ming@users.noreply.github.com> --------- Co-authored-by: LIU ZHE YOU <68415893+jason810496@users.noreply.github.com> Co-authored-by: Guan Ming(Wesley) Chiu <105915352+guan404ming@users.noreply.github.com>
sanederchik
pushed a commit
to sanederchik/airflow
that referenced
this pull request
Jun 7, 2025
* Update TaskLogContent to support virtualized rendering * Update TaskLogPreview and Logs to handle undefined parsedLogs
2 tasks
jose-lehmkuhl
pushed a commit
to jose-lehmkuhl/airflow
that referenced
this pull request
Jul 11, 2025
* Update TaskLogContent to support virtualized rendering * Update TaskLogPreview and Logs to handle undefined parsedLogs
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related Issue
#50333
cc @bbovenzi @pierrejeambrun
Why
The log rendering would be quite slow or even crash browser when rendering large logs on the frontend
How
By using @tanstack/react-virtual to support virtualized rendering, I mostly follow this official example to implement. I choose to use dynamic rendering instead of specifying fixed height since our log would have different height.
--> about 7x speed up (for 10000 logs on my local machine and tested 10 times)
Calculated from the moment the browser logs the parsed logs to the console until it’s finally rendered on the screen. Although this isn’t perfectly precise, it still gives a simple metric to confirm that our speed-up trend is positive.
before -> avg 7s
Screen.Recording.2025-05-18.at.3.38.53.PM.mov
after -> avg 1s
Screen.Recording.2025-05-18.at.3.37.58.PM.mov
Minor change: add
acceptfield foruseLog, which make us to supportndjsoneasily by changing the field value and adding parse func for it after it is fully supported in the future.^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.