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.

Sign up for GitHub

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

Jump to bottom

CustomTTYColoredFormatter should inherit TimezoneAware formatter #28439

Merged
merged 2 commits into from
Dec 29, 2022
Merged

CustomTTYColoredFormatter should inherit TimezoneAware formatter #28439

merged 2 commits into from
Dec 29, 2022

Conversation

dstandish
Copy link
Contributor

@dstandish dstandish commented Dec 18, 2022 •
edited
Loading

So that the timestamp format is the same for both.

Before:

[2022-12-18 00:14:38,268] {python.py:178} INFO - Done. Returned value was: None
[2022-12-18 00:14:38,279] {taskinstance.py:1320} INFO - Marking task as SUCCESS. dag_id=example_bash_operator, task_id=hihi, execution_date=20221216T000000, start_date=20221217T225759, end_date=20221218T081438

After:

[2022-12-18T00:17:00.315-0800] {python.py:178} INFO - Done. Returned value was: None
[2022-12-18T00:17:00.328-0800] {taskinstance.py:1320} INFO - Marking task as SUCCESS. dag_id=example_bash_operator, task_id=hihi, execution_date=20221216T000000, start_date=20221217T225759, end_date=20221218T081700

@dstandish dstandish force-pushed the colored-log-output-should-use-same-datetime-formatter branch from d495c14 to d4ed281 Compare December 20, 2022 08:28
@dstandish dstandish requested a review from potiuk December 21, 2022 01:25
kaxil
kaxil reviewed Dec 28, 2022
View reviewed changes
airflow/utils/log/colored_log.py
@@ -38,7 +40,7 @@
BOLD_OFF = esc("22")


class CustomTTYColoredFormatter(TTYColoredFormatter):
class CustomTTYColoredFormatter(TTYColoredFormatter, TimezoneAware):
Copy link
Member

Choose a reason for hiding this comment

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

Want to add any tests for it :) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh forgot to add... sec

@dstandish
CustomTTYColoredFormatter should inherit TimezoneAware formatter
03e974b 
So that the timestamp format is the same for both.
@dstandish dstandish force-pushed the colored-log-output-should-use-same-datetime-formatter branch from d4ed281 to 03e974b Compare December 28, 2022 20:04
@dstandish dstandish added this to the Airflow 2.6.0 milestone Dec 29, 2022
@dstandish dstandish requested a review from kaxil December 29, 2022 17:13
@potiuk potiuk merged commit 97da577 into apache:main Dec 29, 2022
@jedcunningham jedcunningham deleted the colored-log-output-should-use-same-datetime-formatter branch December 30, 2022 21:02
@ephraimbuddy ephraimbuddy added the type:improvement Changelog: Improvements label Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@potiuk potiuk potiuk approved these changes

@blag blag blag approved these changes

@kaxil kaxil Awaiting requested review from kaxil

Assignees
No one assigned
Labels
area:logging type:improvement Changelog: Improvements
Projects
None yet
Milestone
Airflow 2.6.0
Development

Successfully merging this pull request may close these issues.
5 participants
@dstandish @potiuk @blag @kaxil @ephraimbuddy