-
Notifications
You must be signed in to change notification settings - Fork 14.8k
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
Render stacktraces properly #47965
Render stacktraces properly #47965
Conversation
for context this is the error at hand
|
Display looks nice, but the indentation of display needs a bit of work. Thanks for picking this up. |
+1 on indentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jens' suggestion looks like the right UX. Let's just rebase
Co-authored-by: Jens Scheffler <[email protected]>
Mind updating the Logs.test.tsx? We should probably add a new test for this stacktrace change |
Yup, trying to get my dev setup working again. Will get fixing once I manage to get that done 🥲 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice addition 🎉
Just a couple of nits, non blocking
airflow-core/src/airflow/ui/src/components/RenderStructuredLog.tsx
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/ui/src/components/RenderStructuredLog.tsx
Outdated
Show resolved
Hide resolved
Fixed the UT which was failing due to a mistaken id, and removed the usage of the quotes as escaped characters and usage of delete
@pierrejeambrun / @bbovenzi all review comments should now be resolved. Could you take another look? |
Co-authored-by: Jens Scheffler <[email protected]>
airflow-core/src/airflow/ui/src/components/renderStructuredLog.tsx
Outdated
Show resolved
Hide resolved
@aritra24 You marked #47965 (comment) as resolved but I still see the statement. Did you push your latest change ? |
Huh, I was sure I deleted it... 🤔 perhaps some commit of mine brought it back. Let me remove it once more... |
It's ok I just pushed it, we can merge when CI is green, thanks |
Thanks! |
* Render stacktraces properly Closes: apache#47921 * Update airflow/ui/src/components/RenderStructuredLog.tsx Co-authored-by: Jens Scheffler <[email protected]> * Fixes ut, resolves comments Fixed the UT which was failing due to a mistaken id, and removed the usage of the quotes as escaped characters and usage of delete * Remove unsed import Co-authored-by: Jens Scheffler <[email protected]> * Remove delete statement --------- Co-authored-by: Jens Scheffler <[email protected]> Co-authored-by: pierrejeambrun <[email protected]>
Closes: #47921
^ 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 newsfragments.