Skip to content
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

Log message source details are grouped #43681

Merged

Conversation

majorosdonat
Copy link
Contributor

@majorosdonat majorosdonat commented Nov 5, 2024

Log message source details are grouped: they are not relevant for most users and can distract them from finding the root cause of their errors.

image


^ 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.

@majorosdonat majorosdonat marked this pull request as ready for review November 5, 2024 09:45
@jscheffl jscheffl added type:bug-fix Changelog: Bug Fixes area:UI Related to UI/UX. For Frontend Developers. legacy ui Whether legacy UI change should be allowed in PR labels Nov 5, 2024
@jscheffl jscheffl added this to the Airflow 2.10.4 milestone Nov 5, 2024
@jscheffl jscheffl requested review from bbovenzi and jscheffl November 5, 2024 09:58
Copy link
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix, looks good for me - also valid to back-port to 2.10 line in my view.

@bbovenzi would it be okay to back-port also for you?

We had a couple of complaints from users who had (1) error markers in logs in UI even marked in red just because the celery worker was not delivering the logs internally - but logs were loaded from file successfully and (2) if logs were archived on blob storage the internal blob storage URL was posted which actually is not accessible for users.

So the first lines posted in the logs mainly confused and were not helpful... Donat came to the proposal to hide/log group them per default.

@majorosdonat majorosdonat marked this pull request as draft November 5, 2024 14:44
@majorosdonat
Copy link
Contributor Author

Moved back to draft until pytests are fixed

@jscheffl jscheffl self-requested a review November 5, 2024 15:40
@bbovenzi
Copy link
Contributor

bbovenzi commented Nov 5, 2024

Makes sense to me!

Majoros Donat (XC-DX/EET2-Bp) added 2 commits November 15, 2024 09:46
@majorosdonat majorosdonat marked this pull request as ready for review November 15, 2024 10:58
@jscheffl jscheffl merged commit 98bda4c into apache:main Nov 15, 2024
81 checks passed
jscheffl pushed a commit to jscheffl/airflow that referenced this pull request Nov 15, 2024
* Log message source details are grouped
* fix static checks
* fix pytests
* Another pytest fix

---------

Co-authored-by: Majoros Donat (XC-DX/EET2-Bp) <[email protected]>
jscheffl pushed a commit to jscheffl/airflow that referenced this pull request Nov 16, 2024
* Log message source details are grouped
* fix static checks
* fix pytests
* Another pytest fix

---------

Co-authored-by: Majoros Donat (XC-DX/EET2-Bp) <[email protected]>
(cherry picked from commit 9d18772)
jscheffl added a commit that referenced this pull request Nov 17, 2024
* Log message source details are grouped (#43681)

* Log message source details are grouped
* fix static checks
* fix pytests
* Another pytest fix

---------

Co-authored-by: Majoros Donat (XC-DX/EET2-Bp) <[email protected]>
(cherry picked from commit 9d18772)

* Fix pytest

---------

Co-authored-by: majorosdonat <[email protected]>
kandharvishnu pushed a commit to kandharvishnu/airflow that referenced this pull request Nov 19, 2024
* Log message source details are grouped
* fix static checks
* fix pytests
* Another pytest fix

---------

Co-authored-by: Majoros Donat (XC-DX/EET2-Bp) <[email protected]>
utkarsharma2 pushed a commit that referenced this pull request Dec 4, 2024
* Log message source details are grouped (#43681)

* Log message source details are grouped
* fix static checks
* fix pytests
* Another pytest fix

---------

Co-authored-by: Majoros Donat (XC-DX/EET2-Bp) <[email protected]>
(cherry picked from commit 9d18772)

* Fix pytest

---------

Co-authored-by: majorosdonat <[email protected]>
utkarsharma2 pushed a commit that referenced this pull request Dec 9, 2024
* Log message source details are grouped (#43681)

* Log message source details are grouped
* fix static checks
* fix pytests
* Another pytest fix

---------

Co-authored-by: Majoros Donat (XC-DX/EET2-Bp) <[email protected]>
(cherry picked from commit 9d18772)

* Fix pytest

---------

Co-authored-by: majorosdonat <[email protected]>
LefterisXefteris pushed a commit to LefterisXefteris/airflow that referenced this pull request Jan 5, 2025
* Log message source details are grouped
* fix static checks
* fix pytests
* Another pytest fix

---------

Co-authored-by: Majoros Donat (XC-DX/EET2-Bp) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:logging area:UI Related to UI/UX. For Frontend Developers. legacy ui Whether legacy UI change should be allowed in PR type:bug-fix Changelog: Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants