You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/06/22 16:17:01 UTC
[GitHub] [pulsar] komalatammal opened a new pull request, #16182: [client][c++] add getLastMessageIdAsync in Consumer
komalatammal opened a new pull request, #16182:
URL: https://github.com/apache/pulsar/pull/16182
### Motivation
Add getLastMessageId method to C++ Consumer.cc to address the missing part in this PR https://github.com/apache/pulsar/pull/15993
### Modifications
Expose methods to get last message Id in consumer, the C++ client's Consumer class
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
*(Please pick either of the following options)*
This change is a trivial rework / code cleanup without any test coverage.
*(or)*
This change is already covered by existing tests, such as *(please describe tests)*.
*(or)*
This change added tests and can be verified as follows:
*(example:)*
- *Added integration tests for end-to-end deployment with large payloads (10MB)*
- *Extended integration test for recovery after broker failure*
### Does this pull request potentially affect one of the following parts:
*If `yes` was chosen, please highlight the changes*
- Dependencies (does it add or upgrade a dependency): (no)
- The public API: (yes)
- The schema: (no)
- The default values of configurations: (no)
- The wire protocol: (no)
- The rest endpoints: (no)
- The admin cli options: (no)
- Anything that affects deployment: (no)
-
### Documentation
Check the box below or label this PR directly.
Need to update docs?
- [ ] `doc-required`
(Your PR needs to update docs and you will update later)
- [ ] `doc-not-needed`
(Please explain why)
- [ ] `doc`
(Your PR contains doc changes)
- [ ] `doc-complete`
(Docs have been already added)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] michaeljmarshall commented on pull request #16182: [client][c++] add getLastMessageIdAsync in Consumer
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on PR #16182:
URL: https://github.com/apache/pulsar/pull/16182#issuecomment-1208862724
> yes, it is a strict constraint for all non-trivial changes.
@eolivelli - I did not realize this was a strict requirement. Have we discussed this on the dev mailing list recently? It might be worth re-iterating the requirement. It might also be worth documenting here: https://github.com/apache/pulsar/wiki/Committer-Guide.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #16182: [client][c++] add getLastMessageIdAsync in Consumer
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #16182:
URL: https://github.com/apache/pulsar/pull/16182#issuecomment-1163328585
@komalatammal Please provide a correct documentation label for your PR.
Instructions see [Pulsar Documentation Label Guide](https://docs.google.com/document/d/1Qw7LHQdXWBW9t2-r-A7QdFDBwmZh6ytB4guwMoXHqc0).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] BewareMyPower commented on a diff in pull request #16182: [client][c++] add getLastMessageIdAsync in Consumer
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on code in PR #16182:
URL: https://github.com/apache/pulsar/pull/16182#discussion_r904043692
##########
pulsar-client-cpp/include/pulsar/Consumer.h:
##########
@@ -390,6 +390,17 @@ class PULSAR_PUBLIC Consumer {
*/
bool isConnected() const;
+ /**
+ * Asynchronously get an ID of the last available message or a message ID with -1 as an entryId if the
+ * topic is empty.
+ */
+ void getLastMessageIdAsync(pulsar::GetLastMessageIdCallback callback);
Review Comment:
```suggestion
void getLastMessageIdAsync(GetLastMessageIdCallback callback);
```
This method definition is already in namespace pulsar.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] BewareMyPower commented on pull request #16182: [client][c++] add getLastMessageIdAsync in Consumer
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on PR #16182:
URL: https://github.com/apache/pulsar/pull/16182#issuecomment-1165270296
@eolivelli But I have a concern that should it be a strict constraint or a blocker? Or how long should we wait if no one has time to review or doesn't want to review? I'm afraid currently there are not enough reviewers for all PRs. For example, https://github.com/apache/pulsar/pull/16072 is another PR I've merged with only 1 binding +1. But no one reviewed this PR in 5 days after it's opened, though I've pinged reviewers again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #16182: [client][c++] add getLastMessageIdAsync in Consumer
Posted by GitBox <gi...@apache.org>.
eolivelli commented on PR #16182:
URL: https://github.com/apache/pulsar/pull/16182#issuecomment-1165255859
@BewareMyPower we need 2 bindings +1 in order to commit a patch.
we should have waited for a second reviewer
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] BewareMyPower commented on pull request #16182: [client][c++] add getLastMessageIdAsync in Consumer
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on PR #16182:
URL: https://github.com/apache/pulsar/pull/16182#issuecomment-1165466238
Okay, I'll keep it in mind next time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] BewareMyPower commented on pull request #16182: [client][c++] add getLastMessageIdAsync in Consumer
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on PR #16182:
URL: https://github.com/apache/pulsar/pull/16182#issuecomment-1165260273
Okay. Got it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] BewareMyPower merged pull request #16182: [client][c++] add getLastMessageIdAsync in Consumer
Posted by GitBox <gi...@apache.org>.
BewareMyPower merged PR #16182:
URL: https://github.com/apache/pulsar/pull/16182
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #16182: [client][c++] add getLastMessageIdAsync in Consumer
Posted by GitBox <gi...@apache.org>.
eolivelli commented on PR #16182:
URL: https://github.com/apache/pulsar/pull/16182#issuecomment-1165455737
yes, it is a strict constraint for all non-trivial changes.
if you want to sponsor a patch the best way is to send a message to dev@ and ask for a second reviewer.
5 days it not much time in a OSS community, we have to keep in mind that committers are volunteers.
there is no hurry in committing a patch usually and we can always ping the community if we think that a patch is really important or urgent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org