Skip to content
This repository has been archived by the owner on Apr 22, 2024. It is now read-only.

Do not disable any logger #430

Merged
merged 3 commits into from May 22, 2017
Merged

Do not disable any logger #430

merged 3 commits into from May 22, 2017

Conversation

cemsbr
Copy link
Contributor

@cemsbr cemsbr commented May 18, 2017

Fix #386

List of loggers that were disabled:

  • concurrent.futures logger
  • kytos.core.buffers logger
  • kytos.core.napps.manager logger
  • kytos.core.switch logger
  • kytos.core.tcp_server logger
  • socketio logger

Fix kytos#386

List of loggers that *were* disabled:
- concurrent.futures logger
- kytos.core.buffers logger
- kytos.core.napps.manager logger
- kytos.core.switch logger
- kytos.core.tcp_server logger
- socketio logger
@cemsbr cemsbr requested a review from macartur May 18, 2017 21:38
@@ -23,7 +23,7 @@ def load_logging_file(cls, logging_file):
cls.configuration.read(logging_file)

try:
config.fileConfig(logging_file)
config.fileConfig(logging_file, disable_existing_loggers=False)
except FileNotFoundError:
cls.configuration.set('handler_syslog', 'args', '[]')
config.fileConfig(cls.configuration)
Copy link
Member

Choose a reason for hiding this comment

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

Should not the line be here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch. I refactored not to repeat code.

- Review of commit 102481 ("Do not disable any logger", PR kytos#430 to fix kytos#386)
  - Add argument also to another fileConfig call.
- LogManager refactoring
  - Add tests
  - Not raising exception if there's no logging.ini file

Signed-off-by: Diego Rabatone Oliveira <diraol@ncc.unesp.br>
try:
config.fileConfig(logging_file)
except FileNotFoundError:
cls.configuration.set('handler_syslog', 'args', '[]')
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess you forgot to verify the handler_syslog. This line change the 'handler_syslog' if the file /dev/syslog don't exists.

Some details:
- Preventing errors when no syslog is available
- LogManager refactoring
- Added many comments and tests (100% coverage of logs module)
- Unified 2 files that was testing logs module

Signed-off-by: Diego Rabatone Oliveira <diraol@ncc.unesp.br>
@cemsbr
Copy link
Contributor Author

cemsbr commented May 20, 2017

@beraldoleal and @macartur, I believe all the issues are solved now. There's also tests for this issues and 100% coverage of logs module.

@beraldoleal beraldoleal merged commit 7e91ddd into kytos:master May 22, 2017
@cemsbr cemsbr deleted the Enable-logs branch May 26, 2017 18:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants