Added more debugging for bug-fixing #48
Conversation
Signed-off-by: Arnau Mora <arnyminer.z@gmail.com>
Signed-off-by: Arnau Mora <arnyminer.z@gmail.com>
Signed-off-by: Arnau Mora <arnyminer.z@gmail.com>
Signed-off-by: Arnau Mora <arnyminer.z@gmail.com>
Signed-off-by: Arnau Mora <arnyminer.z@gmail.com>
| _LOGGER.debug( | ||
| 'Won\'t add device since model "{}" is not in models_list'.format(model) | ||
| ) |
There was a problem hiding this comment.
@leikoilja is model_list still needed?
There was a problem hiding this comment.
Good question, @KapJI.
We have dropped to maintain this list in google-home, do you think we still need it in glocaltokens, @ArnyminerZ?
There was a problem hiding this comment.
Well, I don't know, it might be useful for someone, maybe. If it doesn't bother, I wouldn't remove it, just in case.
There was a problem hiding this comment.
Since we fallback to fetching all devices if no model_list is provided we can just keep it i guess :)
There was a problem hiding this comment.
Reeeally good work, @ArnyminerZ, thanks a lot! 💣
We just need to clean it up a little and it's ready to be used!
Just a little question to both @ArnyminerZ and @KapJI, now this package checks global DEBUG variable flag and only if it's true it will show logging.debug. Is there any near way we can tag along the parent package/component logging level to use here?
Maybe we can pass an extra optional parameter when we initialize client, somewhat like verbose that will set logging level to debug? So then on a parent package/integration (google_home in our example) we can pass verbose based if a user have debug flag set in HA yaml configuration for google_home component?
| _LOGGER.debug( | ||
| 'Won\'t add device since model "{}" is not in models_list'.format(model) | ||
| ) |
There was a problem hiding this comment.
Good question, @KapJI.
We have dropped to maintain this list in google-home, do you think we still need it in glocaltokens, @ArnyminerZ?
glocaltokens/utils/logs.py
Outdated
| """ | ||
| Replaces all the characters in a str by the specified [replace] | ||
|
|
||
| text: The text to censure. | ||
| replace: The character to instead of the content. | ||
| """ | ||
| return replace * len(text) |
There was a problem hiding this comment.
Since we are using this util to print tokens and passwords, we really should add a test to make sure we censor correctly and are not printing any user sensitive data to the log without their consent
|
yeah, passing an extra flag sounds like a way to go. In HA component we can pass current verbosity level. |
Signed-off-by: Arnau Mora <arnyminer.z@gmail.com>
Signed-off-by: Arnau Mora <arnyminer.z@gmail.com>
Signed-off-by: Arnau Mora <arnyminer.z@gmail.com>
Signed-off-by: Arnau Mora <arnyminer.z@gmail.com>
| _LOGGER.debug( | ||
| 'Won\'t add device since model "{}" is not in models_list'.format(model) | ||
| ) |
There was a problem hiding this comment.
Since we fallback to fetching all devices if no model_list is provided we can just keep it i guess :)
|
Looks good to me overall |
Signed-off-by: Arnau Mora <arnyminer.z@gmail.com>
Signed-off-by: Arnau Mora <arnyminer.z@gmail.com>
Signed-off-by: Arnau Mora <arnyminer.z@gmail.com>
Signed-off-by: Arnau Mora <arnyminer.z@gmail.com>
Signed-off-by: Arnau Mora <arnyminer.z@gmail.com>
Signed-off-by: Arnau Mora <arnyminer.z@gmail.com>
Signed-off-by: Arnau Mora <arnyminer.z@gmail.com>
There was a problem hiding this comment.
Looks great to me!
@ArnyminerZ can you please fix failing tests that are broken now after you have added censor method? :)
Also, do you mind us finding a way to pass the verbose parameter her to the GLocalAuthenticationTokens that will set the logger level to debug? I can do that tomorrow if you'd like :)
|
@ArnyminerZ, @KapJI just pushed test fixes and |
Maybe too redundant, but added everything just in case it is useful. Take a look at it.
Closes #47.