Skip to content

Change default log format to include timestamp#5897

Merged
fjetter merged 2 commits intodask:mainfrom
fjetter:timestamp_log_fmt
Mar 9, 2022
Merged

Change default log format to include timestamp#5897
fjetter merged 2 commits intodask:mainfrom
fjetter:timestamp_log_fmt

Conversation

@fjetter
Copy link
Member

@fjetter fjetter commented Mar 3, 2022

Supersedes #3987
Closes #1650

@github-actions
Copy link
Contributor

github-actions bot commented Mar 3, 2022 •
edited
Loading

Unit Test Results

       12 files  ±       0         12 suites  ±0   5h 33m 53s ⏱️ - 1h 26m 48s
  2 623 tests ±       0    2 540 ✔️  -        1    80 💤 ±    0  3 ❌ +2 
12 893 runs   - 2 769  12 253 ✔️  - 2 545  636 💤  - 225  4 ❌ +2 

For more details on these failures, see this check.

Results for commit 73ac0c0. ± Comparison against base commit 39c5e88.

♻️ This comment has been updated with latest results.

@fjetter fjetter force-pushed the timestamp_log_fmt branch from c37485c to 675a977 Compare March 4, 2022 13:55
@fjetter fjetter self-assigned this Mar 4, 2022
@fjetter fjetter force-pushed the timestamp_log_fmt branch from 675a977 to 73ac0c0 Compare March 7, 2022 15:41
bryanwweber
bryanwweber approved these changes Mar 8, 2022
View reviewed changes
Copy link
Contributor

@bryanwweber bryanwweber left a comment

Choose a reason for hiding this comment

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

This looks good to me as far as the code goes; I don't have any visibility into downstream affects of this though.

@fjetter
Copy link
Member Author

fjetter commented Mar 9, 2022

I don't have any visibility into downstream affects of this though.

Even if this does affect anybody downstream negatively, the configuration should allow a very easy compatibility. I would consider it best practice to not use default logging configuration for production deployments anyhow, particularly if you are relying on the logs (e.g. if you are parsing them)

@fjetter fjetter merged commit e1e4385 into dask:main Mar 9, 2022
@fjetter fjetter deleted the timestamp_log_fmt branch March 9, 2022 10:44
@crusaderky crusaderky mentioned this pull request Mar 10, 2022
Closed
crusaderky added a commit to crusaderky/distributed that referenced this pull request Mar 14, 2022
@crusaderky
Revert dask#5897
a319d2f
@fjetter fjetter mentioned this pull request Oct 18, 2022
Open
@hendrikmakait hendrikmakait mentioned this pull request May 17, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gjoseph92 gjoseph92 gjoseph92 approved these changes

+1 more reviewer

@bryanwweber bryanwweber bryanwweber approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@fjetter fjetter

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Logging formatting

3 participants

@fjetter @gjoseph92 @bryanwweber