Renamed error field and improved logging #35
Conversation
present. I changed it so that the warn and error messages will still be output even if the --debug is not present.
and handle_syntax error. In addition, log output now includes the function name, filename, and line number from where the logging information originated. The logging information that is printed to the console also includes a timestamp.
statement is made but nothing is stored in "Syntax Errors" or "Debug". I remedied that. * Further refinements to logging. Mostly cosmetic.
trustymail/cli.py
Outdated
| @@ -37,10 +37,10 @@ | |||
| """ | |||
| from trustymail import __version__ | |||
There was a problem hiding this comment.
While you're shuffling imports around, I would move this one to before the from trustymail import trustymail line below.
There was a problem hiding this comment.
Good idea. I'll make the change.
trustymail/domain.py
Outdated
| # A list of any errors that occurred while scanning records. | ||
| self.errors = [] | ||
| # A list of any debugging information collected while scanning records. | ||
| self.debug = [] |
There was a problem hiding this comment.
I'm a bit leery of the name .debug as that makes me think of a boolean flag more than a list... maybe .debug_info would be more descriptive?
| def handle_error(prefix, domain, error): | ||
| if hasattr(error, "message"): | ||
| if "NXDOMAIN" in error.message and prefix != "[DMARC]": | ||
| def handle_error(prefix, domain, error, syntax_error=False): |
There was a problem hiding this comment.
Can we add a docstring here to explain what the goal / overall effect of this custom handling is?
There was a problem hiding this comment.
Yes. I intend to add docstrings everywhere in trustymail and pshtt at some point, but I'll go ahead and add one for this function.
|
@IanLee1521, thank you for the review! I'll make the changes later this afternoon, once I am free from meetings. |
|
@IanLee1521, I never got free of meetings yesterday so I didn't get to this until today. Please let me know if these changes satisfy your objections. |
trustymail/trustymail.py
Outdated
|
|
||
|
|
||
| def handle_syntax_error(prefix, domain, error): | ||
| """Convenience method for handle_error(prefix, domain, error, |
There was a problem hiding this comment.
@jsf9k -- Minor nit, maybe we should just say "... for handle_error(...)", to shorten the length and to work around any function signature changes later.
Autoupdate pre-commit hooks. Add mypy.
Don't merge this just yet. I'm running a test to make sure this doesn't cause errors withdomain-scan.This looks to be working, so feel free to review and merge.
This should resolve #27.