-
Notifications
You must be signed in to change notification settings - Fork 24
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
Stop empty messages being sent and log warning gracefully #267
base: dev
Are you sure you want to change the base?
Conversation
Codeclimate continually fails due to the extra if statement needed for the log/warning element. Unsure if possible to fix neatly without a function rewrite, so I've put a truly ugly fix here for review as a reminder to rewrite it if needed. |
You've got one of the LDAP commits in here. |
a3dba4f
to
fe4bf07
Compare
fe4bf07
to
f00a96e
Compare
I would go for that fix / function re-write. Something like the following would work, and that would avoid the need for
|
it wouldn't work: Argo_ID is out of scope in that context because of the way you're doing the logs: the argo_id is used only for constructing the log string. So the log string is the only thing passed out of scope Should I just try and rewrite the whole thing? |
I just realised I'm being very silly here! sorry, ignore me, it's totally in scope. However we're resolving to instead just have three separate log message/warning elements rather than upping the complexity with another if...else block. :) |
Resolves #261
Empty messages and files render as empty lines through Dirq but a simple len() > 0 check means that empty files will now return None, and some changes to send_all now gracefully handle None input to log a warning.