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

version logging #4289

Merged
merged 4 commits into from
Nov 19, 2021
Merged

version logging #4289

merged 4 commits into from
Nov 19, 2021

Conversation

nathaniel-may
Copy link
Contributor

@nathaniel-may nathaniel-may commented Nov 17, 2021

Description

Adds versioning to events and does some minor cleanup like making time stamps and pids actually work.

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change

@cla-bot cla-bot bot added the cla:yes label Nov 17, 2021
@nathaniel-may nathaniel-may mentioned this pull request Nov 17, 2021
5 tasks
@nathaniel-may nathaniel-may changed the title start adding version logging, noticed some wrong stuff version logging Nov 17, 2021
@nathaniel-may nathaniel-may mentioned this pull request Nov 17, 2021
26 tasks
@nathaniel-may nathaniel-may marked this pull request as ready for review November 17, 2021 20:15
'ts': e.get_ts(),
'pid': e.get_pid(),
'msg': msg_fn(e),
'level': level if len(level) == 5 else f"{level} "
Copy link
Member

Choose a reason for hiding this comment

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

Since this is for the json output we don't want to format the level here. Just in the text.

Copy link
Contributor

@iknox-fa iknox-fa left a comment

Choose a reason for hiding this comment

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

LGTM!

@iknox-fa iknox-fa merged commit 726aba0 into main Nov 19, 2021
@iknox-fa iknox-fa deleted the v-logging branch November 19, 2021 20:53
@nathaniel-may
Copy link
Contributor Author

Thanks, @emmyoop and @iknox-fa for taking care of this one while I was out!

iknox-fa pushed a commit that referenced this pull request Feb 8, 2022
* start adding version logging, noticed some wrong stuff

* fix bad pid and ts

* remove level format on json logs

Co-authored-by: Emily Rockman <emily.rockman@dbtlabs.com>

automatic commit by git-black, original commits:
  726aba0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants