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/28 02:16:26 UTC
[GitHub] [pulsar] komalatammal opened a new pull request, #16255: [client][python] getLastMessageIdAsync C binding
komalatammal opened a new pull request, #16255:
URL: https://github.com/apache/pulsar/pull/16255
### Motivation
Python function getLastMessageId
It is a C binding for https://github.com/apache/pulsar/pull/16182 to implement get_last_message_id() in Python client.
### Modifications
Add Python/C binding code for get_last_message_id()
### Verifying this change
It compiles.
- [x] Make sure that the change passes the CI checks.
This change is a trivial rework / code cleanup without any test coverage.
### 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)
- [x] `doc-not-needed`
- [x] `doc`
Python Doc will be added
- [ ] `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 #16255: [client][python] getLastMessageIdAsync C binding
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on PR #16255:
URL: https://github.com/apache/pulsar/pull/16255#issuecomment-1208865641
@komalatammal @eolivelli @BewareMyPower - it'd be nice to cherry pick this to 2.11 since it is a feature that is already in the Java client api. Do you think it is too late to get this change into that branch?
--
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 #16255: [client][python] getLastMessageIdAsync C binding
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on PR #16255:
URL: https://github.com/apache/pulsar/pull/16255#issuecomment-1208874947
> My personal answer is no. The feature catch up should not be cherry-picked. For the same reason, if users want some new features of a specific component, they have to upgrade the major version.
I agree with this line of thinking. My only reason for asking in this case is because branch-2.11 was only just recently created and the RC has not yet been tagged. I am not going to ask on the ML because I'll just let this target 2.12.
--
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] zzzming commented on a diff in pull request #16255: [client][python] getLastMessageIdAsync C binding
Posted by GitBox <gi...@apache.org>.
zzzming commented on code in PR #16255:
URL: https://github.com/apache/pulsar/pull/16255#discussion_r909590109
##########
pulsar-client-cpp/README.md:
##########
@@ -280,7 +280,7 @@ ${PULSAR_PATH}/pulsar-test-service-stop.sh
## Requirements for Contributors
-It's recommended to install [LLVM](https://llvm.org/builds/) for `clang-tidy` and `clang-format`. Pulsar C++ client use `clang-format` 6.0+ to format files.
+It's required to install [LLVM](https://llvm.org/builds/) for `clang-tidy` and `clang-format`. Pulsar C++ client use `clang-format` 6.0+ to format files. `make format` will automatically format the files.
Review Comment:
updated
--
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 merged pull request #16255: [client][python] getLastMessageIdAsync C binding
Posted by GitBox <gi...@apache.org>.
michaeljmarshall merged PR #16255:
URL: https://github.com/apache/pulsar/pull/16255
--
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 #16255: [client][python] getLastMessageIdAsync C binding
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #16255:
URL: https://github.com/apache/pulsar/pull/16255#issuecomment-1168137355
@komalatammal Please select only one 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 pull request #16255: [client][python] getLastMessageIdAsync C binding
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on PR #16255:
URL: https://github.com/apache/pulsar/pull/16255#issuecomment-1208870161
@michaeljmarshall I think it needs a discussion. There was a similar discussion here: https://github.com/apache/pulsar/pull/15822#issuecomment-1206270637 /cc @nodece
The core question is that should we cherry-pick all these new features of C++/Python clients to release branches?
My personal answer is no. The feature catch up should not be cherry-picked. For the same reason, if users want some new features of a specific component, they have to upgrade the major version.
Regarding 2.11.0 release, I think you can reply in the mail list. I see branch-2.11 has already been created.
--
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] nodece commented on pull request #16255: [client][python] getLastMessageIdAsync C binding
Posted by GitBox <gi...@apache.org>.
nodece commented on PR #16255:
URL: https://github.com/apache/pulsar/pull/16255#issuecomment-1209035507
Ok, I see.
--
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] nodece commented on pull request #16255: [client][python] getLastMessageIdAsync C binding
Posted by GitBox <gi...@apache.org>.
nodece commented on PR #16255:
URL: https://github.com/apache/pulsar/pull/16255#issuecomment-1208884720
@BewareMyPower @michaeljmarshall Thanks for your explanation! I have a question about the broker and client of old version, if the broker supports the xx feature, but the client doesn't support it, shouldn't we cherry-pick 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 commented on pull request #16255: [client][python] getLastMessageIdAsync C binding
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on PR #16255:
URL: https://github.com/apache/pulsar/pull/16255#issuecomment-1208926338
> if the broker supports the xx feature, but the client doesn't support it
IMO no as I've said:
> The feature catch up should not be cherry-picked.
--
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 #16255: [client][python] getLastMessageIdAsync C binding
Posted by GitBox <gi...@apache.org>.
michaeljmarshall commented on PR #16255:
URL: https://github.com/apache/pulsar/pull/16255#issuecomment-1208936875
> @BewareMyPower @michaeljmarshall Thanks for your explanation! I have a question about the broker and client of old version, if the broker supports the xx feature, but the client doesn't support it, shouldn't we cherry-pick it?
One reason I think it is unnecessary to backport features to older versions of the client is that the protocol has a feature handshake built in that is supposed to enable different versions of the clients and servers to work together seamlessly. Therefore, if a user wants to get a new feature in the client that was already supported on the broker in an older version, they should be able to upgrade without issue. That being said, I am not sure that we document the client/server compatibility anywhere and I am not sure how good the test coverage is for the many permutations of client versions.
--
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] zzzming commented on a diff in pull request #16255: [client][python] getLastMessageIdAsync C binding
Posted by GitBox <gi...@apache.org>.
zzzming commented on code in PR #16255:
URL: https://github.com/apache/pulsar/pull/16255#discussion_r909590109
##########
pulsar-client-cpp/README.md:
##########
@@ -280,7 +280,7 @@ ${PULSAR_PATH}/pulsar-test-service-stop.sh
## Requirements for Contributors
-It's recommended to install [LLVM](https://llvm.org/builds/) for `clang-tidy` and `clang-format`. Pulsar C++ client use `clang-format` 6.0+ to format files.
+It's required to install [LLVM](https://llvm.org/builds/) for `clang-tidy` and `clang-format`. Pulsar C++ client use `clang-format` 6.0+ to format files. `make format` will automatically format the files.
Review Comment:
updated
--
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] Anonymitaet commented on a diff in pull request #16255: [client][python] getLastMessageIdAsync C binding
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on code in PR #16255:
URL: https://github.com/apache/pulsar/pull/16255#discussion_r909029902
##########
pulsar-client-cpp/README.md:
##########
@@ -280,7 +280,7 @@ ${PULSAR_PATH}/pulsar-test-service-stop.sh
## Requirements for Contributors
-It's recommended to install [LLVM](https://llvm.org/builds/) for `clang-tidy` and `clang-format`. Pulsar C++ client use `clang-format` 6.0+ to format files.
+It's required to install [LLVM](https://llvm.org/builds/) for `clang-tidy` and `clang-format`. Pulsar C++ client use `clang-format` 6.0+ to format files. `make format` will automatically format the files.
Review Comment:
```suggestion
It's required to install [LLVM](https://llvm.org/builds/) for `clang-tidy` and `clang-format`. Pulsar C++ client use `clang-format` 6.0+ to format files. `make format` automatically formats the files.
```
Write in the simple present tense as much as possible if you are covering facts that were, are, and forever shall be true.
https://docs.google.com/document/d/1lc5j4RtuLIzlEYCBo97AC8-U_3Erzs_lxpkDuseU0n4/edit#bookmark=id.e8uqh1awkcnp
--
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