Skip to content
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

Negative acks in C++ client #3750

Merged
merged 5 commits into from Mar 13, 2019
Merged

Negative acks in C++ client #3750

merged 5 commits into from Mar 13, 2019

Conversation

merlimat
Copy link
Contributor

@merlimat merlimat commented Mar 5, 2019

Motivation

Similar to #3703, adding support for negative Acks in C++ client (and C wrapper)

@merlimat merlimat added type/feature The PR added a new feature or issue requested a new feature component/c++ labels Mar 5, 2019
@merlimat merlimat added this to the 2.4.0 milestone Mar 5, 2019
@merlimat merlimat self-assigned this Mar 5, 2019
Copy link
Contributor

@jerrypeng jerrypeng left a comment

Choose a reason for hiding this comment

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

@merlimat are you planning on exposing the neg ack API for python client in a subsequent PR?

@jerrypeng
Copy link
Contributor

rerun java8 tests

@merlimat
Copy link
Contributor Author

merlimat commented Mar 5, 2019

@merlimat are you planning on exposing the neg ack API for python client in a subsequent PR?

Yes, I have pending PRs for Python and Go that depend on this one

* @return the consumer builder instance
*/
void setNegativeAckRedeliveryDelayMs(long redeliveryDelayMillis);
long getNegativeAckRedeliveryDelayMs() const;
Copy link
Member

Choose a reason for hiding this comment

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

Can you add comments for this?

* The {@code Message} to be acknowledged
*/
void pulsar_consumer_negative_acknowledge(pulsar_consumer_t *consumer, pulsar_message_t *message);
void pulsar_consumer_negative_acknowledge_id(pulsar_consumer_t *consumer, pulsar_message_id_t *messageId);
Copy link
Member

Choose a reason for hiding this comment

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

comments?

*/
void pulsar_configure_set_negative_ack_redelivery_delay_ms(
pulsar_consumer_configuration_t *consumer_configuration, long redeliveryDelayMillis);
long pulsar_configure_get_negative_ack_redelivery_delay_ms(
Copy link
Member

Choose a reason for hiding this comment

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

comments?

@@ -99,7 +100,7 @@ static void sendCallBackWithDelay(Result r, const Message& msg, std::string pref

class EncKeyReader : public CryptoKeyReader {
private:
void readFile(std::string fileName, std::string& fileContents) const {
void readFile(std::string fileName, std::string &fileContents) const {
Copy link
Contributor

Choose a reason for hiding this comment

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

const std::string& fileName

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is existing code and it was just one space reformatted by make format.
In any case the functional change should be done in a different PR. Given this is for test, I'm not particular concerned about a string copy when loading the key file.

Copy link
Contributor

@jai1 jai1 left a comment

Choose a reason for hiding this comment

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

@jai1
Copy link
Contributor

jai1 commented Mar 6, 2019

rerun java8 tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feature The PR added a new feature or issue requested a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants