-
Notifications
You must be signed in to change notification settings - Fork 13
Added code to write to STDERR when no results found #22
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
Conversation
@@ -144,6 +145,8 @@ def handle_response(response): | |||
events = response_dict.get(u"fileEvents") | |||
for event in events: | |||
output_logger.info(event) | |||
if not len(events): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will print for every page of response. Also, if the last page has 0 results, and the first page had 10000 results, this will also print this error message.
What we need to do is something similar to the variable global _EXCEPTIONS_OCCURRED
at lie 132.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, it won't print for every page of response, just the last one if it was empty, which is still unexpected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed, used global variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only print the error once and at the right time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wait, could just add a CHANGELOG entry for this? With the release version, just do ## Unreleased
.
We need a CHANGELOG entry for this too |
Added changelog. |
for event in events: | ||
output_logger.info(event) | ||
|
||
if not _TOTAL_EVENTS: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will work but it is a little confusing.
Can you move this to the _handle_result()
function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything in handle_response
should just be page-relevant stuff. Side-effects are a necessary evil right now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed.
@@ -170,6 +173,9 @@ def _verify_compatibility_with_advanced_query(key, val): | |||
def _handle_result(): | |||
if is_interactive() and _EXCEPTIONS_OCCURRED: | |||
print_error(u"View exceptions that occurred at [HOME]/.code42cli/log/code42_errors.") | |||
global _TOTAL_EVENTS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The global
here is not really needed but that is ok
e59aef0
to
50b9b50
Compare
Tested method for
print
, expecting behaviour to be consistent forsend-to
andwrite-to
.Did not add tests, as it is writing to STDERR