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

GSK-2822 Don't edit global logger level #1806

Merged
merged 7 commits into from
Feb 21, 2024
Merged

GSK-2822 Don't edit global logger level #1806

merged 7 commits into from
Feb 21, 2024

Conversation

kevinmessiaen
Copy link
Member

Description

Updated logger to only update giskard log level to INFO by default

Related Issue

Type of Change

  • 📚 Examples / docs / tutorials / dependencies update
  • 🔧 Bug fix (non-breaking change which fixes an issue)
  • 🥂 Improvement (non-breaking change which improves an existing feature)
  • 🚀 New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to change)
  • 🔐 Security fix

Copy link

linear bot commented Feb 19, 2024



def test_root_log_level_default_warning():
assert logging.root.level == logging.WARNING, 'Root package log level should be set to default "WARNING"'
Copy link
Member

Choose a reason for hiding this comment

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

I'm all for this modification, but for scan and worker, we should have some logs, no ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes you're right, it should be an arg for the CLI to allow flexibility

)

logging.root.setLevel(logging.getLevelName(log_level))
Copy link
Member

Choose a reason for hiding this comment

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

Setting log level does not work for me : if I set it to error, I still have logs from giskard weirdly. Not sure why

Copy link
Member

Choose a reason for hiding this comment

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

Normally setting Error level to root level should suppress every log even from giskard, in my case worker is still starting with logs.

Copy link
Member

Choose a reason for hiding this comment

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

Nvm, it was only at start, seems ok after

@Hartorn Hartorn enabled auto-merge (squash) February 21, 2024 09:30
Copy link

sonarcloud bot commented Feb 21, 2024

@Hartorn Hartorn merged commit e6112f6 into main Feb 21, 2024
16 checks passed
@Hartorn Hartorn deleted the GSK-2822 branch February 21, 2024 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants