-
Notifications
You must be signed in to change notification settings - Fork 15.6k
Update TaskLogContent
to support virtualized rendering
#50746
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Cool! I feel not confident to review but very great improvement! |
e59c858
to
f8233fb
Compare
Thanks! I'm still trying to figure out how to test virtualized list. |
Nice change! Good job! |
f8233fb
to
2ea9bb8
Compare
airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/Logs.test.tsx
Outdated
Show resolved
Hide resolved
2ea9bb8
to
adfb2f0
Compare
3e1bc49
to
2a9eb7b
Compare
airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/TaskLogContent.tsx
Show resolved
Hide resolved
airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/Logs.tsx
Outdated
Show resolved
Hide resolved
2a9eb7b
to
e390fb4
Compare
e390fb4
to
c9c0cfb
Compare
c9c0cfb
to
2f98bea
Compare
airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/TaskLogContent.tsx
Outdated
Show resolved
Hide resolved
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. |
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 |
…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 <[email protected]>
Thanks! |
Big thanks @guan404ming 🙌 That's a fantastic improvement, really appreciate your work on this! |
…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 <[email protected]>
…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 <[email protected]> --------- Co-authored-by: LIU ZHE YOU <[email protected]> Co-authored-by: Guan Ming(Wesley) Chiu <[email protected]>
…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 <[email protected]> --------- Co-authored-by: LIU ZHE YOU <[email protected]> Co-authored-by: Guan Ming(Wesley) Chiu <[email protected]>
* Update TaskLogContent to support virtualized rendering * Update TaskLogPreview and Logs to handle undefined parsedLogs
* Update TaskLogContent to support virtualized rendering * Update TaskLogPreview and Logs to handle undefined parsedLogs
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
accept
field foruseLog
, which make us to supportndjson
easily 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.rst
or{issue_number}.significant.rst
, in airflow-core/newsfragments.