Skip to content
This repository has been archived by the owner on May 17, 2021. It is now read-only.

[Nest] Quiesce logging on status poll #3111

Merged
merged 1 commit into from
Sep 2, 2015

Conversation

watou
Copy link
Contributor

@watou watou commented Aug 31, 2015

If there is an error polling the Nest API (usually due to network issues), the failed poll would log as an ERROR and dump a stacktrace to openhab.log which is typically unhelpful. Since network issues are usually temporary, this PR changes the log to WARN and the stacktrace is now not dumped (only the exception message is logged). However, at the DEBUG level the stacktrace is dumped to the log for the non-typical failure situation. @mrguessed, do you agree that this would be better?

@teichsta teichsta added this to the 1.8.0 milestone Aug 31, 2015
@mrguessed
Copy link
Contributor

Ok with making it warn, instead of error, but would prefer to see only one log line in either case.

More like

if debug
   log.debug(...)
else
   log.warn(...)

It prevents log interleaving issues in multi-threaded envs and ensures there's only one log line per 'event'. Not a hard rule but more a preference (vs 2 log lines in log file)

@watou
Copy link
Contributor Author

watou commented Aug 31, 2015

Thanks, Mark. Good point. How about this construct?

            if (logger.isDebugEnabled()) {
                logger.warn("Exception reading from Nest.", e);             
            } else {
                logger.warn("Exception reading from Nest: {}", e.getMessage());             
            }

@mrguessed
Copy link
Contributor

Sure, as long as the logger, at warn level, emits the stack then that's the right construct...

@watou watou force-pushed the nest-quiesce-logging branch from 99827eb to c4e6e35 Compare September 2, 2015 06:19
watou added a commit that referenced this pull request Sep 2, 2015
[Nest] Quiesce logging on status poll
@watou watou merged commit e15032c into openhab:master Sep 2, 2015
@watou watou deleted the nest-quiesce-logging branch September 2, 2015 06:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants