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

fix: A less-than/greater-than error in the daist stdout logging filter. #10242

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

highvelcty
Copy link
Contributor

@highvelcty highvelcty commented Jan 10, 2025

PR TITLE (Commit Body)

Fix: A less-than/greater-than error in the daist stdout logging filter.

Ticket

N/A

Description

There is a standard output filter implemented for the daist testing framework that had a logical error in it. The filter is inherited from the logging.Filter class, overriding the filter method. This method should return False if a logging record should be filtered (i.e. not output to standard out). The incorrect logic was returning True.

Test Plan

image

Checklist

  • [x ] Changes have been manually QA'd
  • [?] New features have been approved by the corresponding PM
  • User-facing API changes have the "User-facing API Change" label
  • [ let me know if needed] Release notes have been added as a separate file under docs/release-notes/
    See Release Note for details.
  • [x ] Licenses have been included for new code which was copied and/or modified from any external code

@highvelcty highvelcty requested a review from a team as a code owner January 10, 2025 17:45
Copy link

cla-bot bot commented Jan 10, 2025

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @highvelcty on file. In order for us to review and merge your code, please start the CLA process at https://determined.ai/cla.

After we approve your CLA, we will update the contributors list (private) and comment @cla-bot[bot] check to rerun the check.

@highvelcty
Copy link
Contributor Author

@mackrorysd , good to see you again in code!

@highvelcty highvelcty changed the title Fixed a less-than/greater-than error in the daist stdout logging filter. Fix: A less-than/greater-than error in the daist stdout logging filter. Jan 10, 2025
@highvelcty highvelcty changed the title Fix: A less-than/greater-than error in the daist stdout logging filter. fix: A less-than/greater-than error in the daist stdout logging filter. Jan 17, 2025
@mackrorysd mackrorysd closed this Jan 22, 2025
@mackrorysd mackrorysd reopened this Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants