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

Isolate manifester's logger #44

Merged
merged 1 commit into from
Jul 18, 2024

Conversation

ogajduse
Copy link
Member

@ogajduse ogajduse commented Jul 17, 2024

Broker and Manifester both use the default logger, so the last to have their log setup run overrides the other.

This PR isolates the manifester's own logger in its library, but it keeps the possibility to modify the logger from outside using the setup_logzero API.

@ogajduse ogajduse marked this pull request as ready for review July 17, 2024 18:31
Copy link
Collaborator

@synkd synkd left a comment

Choose a reason for hiding this comment

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

Looks good to me. I tested locally with both the Manifester CLI and with Robottelo. The Manifester CLI logs as expected to logs/manifester.log, and Manifester actions in Robottelo log as expected to logs/robottelo.log.

@synkd synkd requested a review from JacobCallahan July 17, 2024 19:49
Copy link
Member

@JacobCallahan JacobCallahan left a comment

Choose a reason for hiding this comment

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

Nice!

@ogajduse ogajduse force-pushed the fix/use-isolated-logger branch from 4eed2b1 to 687800e Compare July 17, 2024 21:44
@ogajduse ogajduse force-pushed the fix/use-isolated-logger branch from 687800e to 652b432 Compare July 18, 2024 15:04
@synkd synkd merged commit 4401c8a into SatelliteQE:master Jul 18, 2024
4 checks passed
@ogajduse ogajduse deleted the fix/use-isolated-logger branch July 19, 2024 09:04
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