-
Notifications
You must be signed in to change notification settings - Fork 99
samples: exit early if no messages are returned #989
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
| // Exit the program if the pull response is empty. | ||
| if (pullResponse.getReceivedMessagesList().isEmpty()) { | ||
| System.out.println("No message was pulled. Exiting."); | ||
| System.exit(0); |
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.
System.exit probably isn't what you want here (it may crash the tests) - don't you just want to return?
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.
You're right. Updated to return.
| // Exit the program if the pull response is empty. | ||
| if (pullResponse.getReceivedMessagesList().isEmpty()) { | ||
| System.out.println("No message was pulled. Exiting."); | ||
| System.exit(0); |
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.
same here
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.
Done.
| // Use pullCallable().futureCall to asynchronously perform this operation. | ||
| PullResponse pullResponse = subscriber.pullCallable().call(pullRequest); | ||
|
|
||
| // Exit the program if the pull response is empty. |
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 comment tells me what you are doing (but so does the code). Can we update it to tell me why we are doing?
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.
That's a good point. Updated.
* samples: exit early if no messages are pulled * address kurtis's comments * update comment
Fixes #189
The current samples will fail with
io.grpc.StatusRuntimeException: INVALID_ARGUMENT: You have not specified an ack ID in the requestif no messages are returned. Updating them to handle this case.This does not affect the existing tests for the samples.
I also noticed something that should not have been published with one of the samples in the first place - my project and topic ID. Apologies.