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

[WIP] [3006.x] Add once option to logging so we stop spamming logs with useless info #66826

Merged
merged 1 commit into from
Sep 13, 2024

Conversation

dwoz
Copy link
Contributor

@dwoz dwoz commented Aug 21, 2024

What does this PR do?

What issues does this PR fix or reference?

Fixes

Previous Behavior

Remove this section if not relevant

New Behavior

Remove this section if not relevant

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes/No

Please review Salt's Contributing Guide for best practices, including the
PR Guidelines.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@dwoz dwoz requested a review from a team as a code owner August 21, 2024 22:24
@salt-project-bot-prod-environment salt-project-bot-prod-environment bot changed the title [wip] Add once option to logging so we stop spamming logs with useless info [3006.x][wip] Add once option to logging so we stop spamming logs with useless info Aug 21, 2024
@dwoz dwoz changed the title [3006.x][wip] Add once option to logging so we stop spamming logs with useless info [WIP] [3006.x] Add once option to logging so we stop spamming logs with useless info Aug 21, 2024
Comment on lines +245 to +247
if str(args) in self.ONCECACHE:
return
self.ONCECACHE.add(str(args))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is going to prevent the str from being garbage collected which could be a problem for long running processes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the strings are cached forever. This won't be an endlessly growing cache though. As of now I've only identified one place in the code where this feature is really needed. So, as long as the once option is used sparingly I think it's okay.

@dwoz dwoz requested a review from bdrx312 September 10, 2024 21:03
@dwoz dwoz merged commit 468b26e into saltstack:3006.x Sep 13, 2024
185 of 186 checks passed
@dwoz dwoz self-assigned this Sep 13, 2024
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.

3 participants