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

Fixes #35699 - Introduce a parameter to configure logging #226

Merged
merged 1 commit into from
Nov 7, 2022

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Nov 1, 2022

This came up via theforeman/foreman-documentation#1715 where there are instructions to manually configure this. The goal is that users don't touch individual files but rather use the installer to modify it.

I do wonder about https://hibernate.atlassian.net/browse/HHH-12927. It's been resolved for 4 years now so is it still needed?

<%- unless 'org.candlepin' in $loggers { -%>

# uncomment to enable debug logging in candlepin.log:
#log4j.logger.org.candlepin=DEBUG
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to keep this comment here that a user can't do anything with? Or would lead them astray?

Copy link
Member Author

Choose a reason for hiding this comment

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

My reasoning was that it can help users who quickly want to debug, but I wasn't quite sure.

Copy link
Member

Choose a reason for hiding this comment

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

I'd lean to drop to prevent the inevitable bug reports about this gets wiped out every time they run the installer and then having to explain they ought to use the installer itself.

@ekohl
Copy link
Member Author

ekohl commented Nov 4, 2022

I do wonder about https://hibernate.atlassian.net/browse/HHH-12927. It's been resolved for 4 years now so is it still needed?

@ehelms any idea about this?

@ehelms
Copy link
Member

ehelms commented Nov 4, 2022

I do wonder about https://hibernate.atlassian.net/browse/HHH-12927. It's been resolved for 4 years now so is it still needed?

@ehelms any idea about this?

No direct knowledge, but it certainly reads like it was fixed and therefore should no longer be something we need to work around. Drop it.

@evgeni
Copy link
Member

evgeni commented Nov 4, 2022

Looking at the ticket, it's said to be fixed in hibernate 5.4, and a Katello 4.5 that I have here has /var/lib/tomcat/webapps/candlepin/WEB-INF/lib/hibernate-core-5.4.9.Final.jar in candlepin-4.1.11-1.el8.noarch, so I guess we're fine

This also drops the Hibernate logger since it should have been resolved
by now.
@ekohl
Copy link
Member Author

ekohl commented Nov 4, 2022

That greatly simplified the whole PR.

@@ -26,9 +26,9 @@ candlepin.ca_key_password=<%= scope['candlepin::ca_key_password_unsensitive'] %>
<%- end -%>

candlepin.async.jobs.ExpiredPoolsCleanupJob.schedule=<%= scope['candlepin::expired_pools_schedule'] %>
<% if scope['candlepin::loggers'].any? -%>
Copy link
Member

Choose a reason for hiding this comment

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

I am surprised this any? is needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Mostly for a visual separation with an empty line between the groups.

@ekohl ekohl merged commit 44c6333 into theforeman:master Nov 7, 2022
@ekohl ekohl deleted the 35699-logging branch November 7, 2022 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants