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

ipatests: keep default log level for 389-ds #1103

Closed
wants to merge 1 commit into from

Conversation

nicki-krizek
Copy link
Contributor

@nicki-krizek nicki-krizek commented Sep 20, 2017

The log level of 8192 for 389-ds is very noisy and conceals useful debug
messages. The default log level 0 should be sufficient for our purposes.

Part of: https://pagure.io/freeipa/issue/7162
Signed-off-by: Tomas Krizek tkrizek@redhat.com

@nicki-krizek
Copy link
Contributor Author

@tbordaz Could you please review/ack? You can check any of the ds logs and verify the debug messages aren't there.

Copy link
Contributor

@tbordaz tbordaz left a comment

Choose a reason for hiding this comment

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

The patch is fine to me. It could be an option (to keep good code) to change enable_replication_debugging into a generic function taking a log-level in param. Then calling it with level 0.

@nicki-krizek
Copy link
Contributor Author

I agree it's better to keep the code. I've changed to PR to use log_level of 0 by default, but left the option to change it.

@nicki-krizek nicki-krizek added the re-run Trigger a new run of PR-CI label Sep 21, 2017
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Sep 21, 2017
@rcritten rcritten added the ack Pull Request approved, can be merged label Sep 22, 2017
@rcritten
Copy link
Contributor

LGTM too

@nicki-krizek nicki-krizek removed the ack Pull Request approved, can be merged label Sep 26, 2017
@nicki-krizek
Copy link
Contributor Author

@Firstyear
Copy link
Contributor

Hmmm, maybe there is a test elsewhere that is causing this change? It looks like the loglevel is set correctly and the server initial is not logging, but suddenly it changes. You could use the DS audit log to try and track who triggered the change of the loglevel, but I think this patch and what it is "now" is good to merge.

During integration tests, the log level of 8192 (replication debugging)
was excessive and made reading 389-ds logs very hard without providing
any useful information.

Part of: https://pagure.io/freeipa/issue/7162
Signed-off-by: Tomas Krizek <tkrizek@redhat.com>
@nicki-krizek nicki-krizek added the ack Pull Request approved, can be merged label Oct 23, 2017
@nicki-krizek
Copy link
Contributor Author

Based on comments above, I'm re-adding the ack. I'll keep the ticket open until the replication messages are no longer in ds log.

@nicki-krizek nicki-krizek added the pushed Pull Request has already been pushed label Oct 23, 2017
@nicki-krizek
Copy link
Contributor Author

master:

  • be6f1a6 ipatests: set default 389-ds log level to 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants