Skip to content

Conversation

@anguillanneuf
Copy link
Contributor

@anguillanneuf anguillanneuf commented Feb 3, 2022

Fixes #189

The current samples will fail with io.grpc.StatusRuntimeException: INVALID_ARGUMENT: You have not specified an ack ID in the request if 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.

@product-auto-label product-auto-label bot added api: pubsub Issues related to the googleapis/java-pubsub API. samples Issues that are directly related to samples. labels Feb 3, 2022
@anguillanneuf anguillanneuf changed the title samples: check for no messages case in pull sample to avoid error out samples: check for no messages case in pull sample Feb 3, 2022
@anguillanneuf anguillanneuf changed the title samples: check for no messages case in pull sample samples: exit early if no messages are pulled Feb 3, 2022
@anguillanneuf anguillanneuf requested review from a team and mmicatka February 3, 2022 00:51
@anguillanneuf anguillanneuf changed the title samples: exit early if no messages are pulled samples: exit early if no messages are returned Feb 3, 2022
// Exit the program if the pull response is empty.
if (pullResponse.getReceivedMessagesList().isEmpty()) {
System.out.println("No message was pulled. Exiting.");
System.exit(0);
Copy link

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?

Copy link
Contributor Author

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);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

Copy link
Contributor Author

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.
Copy link

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?

Copy link
Contributor Author

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.

@anguillanneuf anguillanneuf merged commit ab828a0 into main Feb 3, 2022
@anguillanneuf anguillanneuf deleted the pull branch February 3, 2022 18:21
rajanya-google pushed a commit to rajanya-google/java-pubsub that referenced this pull request Feb 16, 2022
* samples: exit early if no messages are pulled * address kurtis's comments * update comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: pubsub Issues related to the googleapis/java-pubsub API. samples Issues that are directly related to samples.

2 participants