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

Removing logging exception through update run metadata #1292

Merged
merged 12 commits into from
Jul 9, 2024
Merged

Removing logging exception through update run metadata #1292

merged 12 commits into from
Jul 9, 2024

Conversation

jjanezhang
Copy link
Contributor

@jjanezhang jjanezhang commented Jun 20, 2024 •
edited
Loading

We are moving to catching exceptions through the python excepthook override 7/8/24. We can remove logging to metadata then.

@jjanezhang jjanezhang requested a review from a team as a code owner June 20, 2024 00:06
@jjanezhang jjanezhang requested a review from mvpatel2000 June 23, 2024 23:17
@jjanezhang jjanezhang mentioned this pull request Jun 23, 2024
Merged
@jjanezhang jjanezhang marked this pull request as draft June 23, 2024 23:28
mvpatel2000
mvpatel2000 approved these changes Jun 24, 2024
View reviewed changes
Copy link
Collaborator

@mvpatel2000 mvpatel2000 left a comment

Choose a reason for hiding this comment

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

LGTM

mvpatel2000
mvpatel2000 approved these changes Jun 24, 2024
View reviewed changes
Copy link
Collaborator

@mvpatel2000 mvpatel2000 left a comment

Choose a reason for hiding this comment

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

LGTM

@jjanezhang jjanezhang requested a review from mvpatel2000 July 8, 2024 20:37
@jjanezhang jjanezhang marked this pull request as ready for review July 8, 2024 20:38
@jjanezhang
Copy link
Contributor Author

Member

removed the logger from a couple more places, requested review again

@jjanezhang jjanezhang mentioned this pull request Jul 8, 2024
Merged
irenedea
irenedea approved these changes Jul 8, 2024
View reviewed changes
Copy link
Contributor

@irenedea irenedea left a comment

Choose a reason for hiding this comment

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

LGTM! minor comment

llmfoundry/callbacks/run_timeout_callback.py Outdated Show resolved Hide resolved
@jjanezhang jjanezhang merged commit 304bf28 into main Jul 9, 2024
9 checks passed
@dakinggg dakinggg deleted the jane/remove-logger-exception branch August 6, 2024 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mvpatel2000 mvpatel2000 mvpatel2000 approved these changes

@irenedea irenedea irenedea approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jjanezhang @irenedea @mvpatel2000