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/11/17 10:08:36 UTC
[GitHub] [pulsar] BewareMyPower opened a new pull request, #18519: [improve][client] Move acknowledge APIs to another interface and improve docs
BewareMyPower opened a new pull request, #18519:
URL: https://github.com/apache/pulsar/pull/18519
### Motivation
Currently there are many acknowledge methods in `Consumer` interface, including:
- 7 overloads of `acknowledgeAsync`
- 4 overloads of `acknowledge`
- 3 overloads of `acknowledgeCumulativeAsync`
- 2 overloads of `acknowledgeCumulative`
The JavaDocs of these APIs are also massive and duplicated. For example, some terms are not for application users like "pending ack", which should not appear in API Docs.
### Modifications
- Add a new interface `MessageAcknowledger` and move the acknowledge APIs into this new interface.
- Improve the API docs. For some overload methods, change them to default methods so that duplicated description can be avoided.
The new API docs should be much more clear than before. Though this PR adds a new API, the API compatibility is guaranteed.
### Does this pull request potentially affect one of the following parts:
*If the box was checked, please highlight the changes*
- [ ] Dependencies (add or upgrade a dependency)
- [x] The public API
- [ ] The schema
- [ ] The default values of configurations
- [ ] The threading model
- [ ] The binary protocol
- [ ] The REST endpoints
- [ ] The admin CLI options
- [ ] Anything that affects deployment
### Documentation
- [x] `doc` <!-- Your PR contains doc changes. Please attach the local preview screenshots (run `sh start.sh` at `pulsar/site2/website`) to your PR description, or else your PR might not get merged. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
### Matching PR in forked repository
PR in forked repository: https://github.com/BewareMyPower/pulsar/pull/9
--
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 #18519: [improve][client] Move acknowledge APIs to another interface and improve docs
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on PR #18519:
URL: https://github.com/apache/pulsar/pull/18519#issuecomment-1318589146
> we must ensure that changing the API jars doesn't break compatibility and requires rebuilding the applications
Is there any way to verify 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 #18519: [improve][client] Move acknowledge APIs to another interface and improve docs
Posted by GitBox <gi...@apache.org>.
BewareMyPower merged PR #18519:
URL: https://github.com/apache/pulsar/pull/18519
--
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 #18519: [improve][client] Move acknowledge APIs to another interface and improve docs
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on PR #18519:
URL: https://github.com/apache/pulsar/pull/18519#issuecomment-1318768693
@eolivelli I wrote a function just now with the 2.10.2 dependency.
```java
package org.example;
import org.apache.pulsar.functions.api.Context;
import org.apache.pulsar.functions.api.Function;
public class StringLengthExample implements Function<String, Integer> {
@Override
public Integer process(String input, Context context) throws Exception {
return input.length();
}
}
```
```xml
<dependency>
<groupId>org.apache.pulsar</groupId>
<artifactId>pulsar-functions-api</artifactId>
<version>2.10.2</version>
</dependency>
```
Then I created the function in the branch of this PR and it works well.
```bash
./bin/pulsar-admin functions create \
--jar ./PulsarFunctionDemo-1.0-SNAPSHOT.jar \
--classname org.example.StringLengthExample \
--tenant public \
--namespace default \
--name word-count \
--inputs persistent://public/default/sentences \
--output persistent://public/default/count
```
```bash
./bin/pulsar-client produce sentences -m "hello world java"
```
```bash
$ ./bin/pulsar-client consume -s sub count -n 0
...
----- got message -----
key:[null], properties:[__pfn_input_msg_id__=CAsQADAA, __pfn_input_topic__=persistent://public/default/sentences], content:16
```
Could it verify the API and binary compatibility?
--
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] tisonkun commented on pull request #18519: [improve][client] Move acknowledge APIs to another interface and improve docs
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #18519:
URL: https://github.com/apache/pulsar/pull/18519#issuecomment-1326124962
/pulsarbot run-failure-checks
--
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 #18519: [improve][client] Move acknowledge APIs to another interface and improve docs
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on PR #18519:
URL: https://github.com/apache/pulsar/pull/18519#issuecomment-1318956713
I added the Consumer related API invocations, it also worked well.
```java
final PulsarClient client = PulsarClient.builder().serviceUrl("pulsar://localhost:6650").build();
final Consumer<String> consumer = client.newConsumer(Schema.STRING)
.topic("xxx")
.subscriptionName("sub")
.subscribe();
consumer.acknowledge(MessageId.earliest);
consumer.close();
client.close();
return input.length();
```
--
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] codecov-commenter commented on pull request #18519: [improve][client] Move acknowledge APIs to another interface and improve docs
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #18519:
URL: https://github.com/apache/pulsar/pull/18519#issuecomment-1326174571
# [Codecov](https://codecov.io/gh/apache/pulsar/pull/18519?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#18519](https://codecov.io/gh/apache/pulsar/pull/18519?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bd2b96f) into [master](https://codecov.io/gh/apache/pulsar/commit/545f33fa35309510585e28283ba485a60deddf15?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (545f33f) will **increase** coverage by `2.54%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/pulsar/pull/18519?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #18519 +/- ##
============================================
+ Coverage 43.96% 46.51% +2.54%
+ Complexity 10358 9093 -1265
============================================
Files 757 617 -140
Lines 72773 58514 -14259
Branches 7818 6099 -1719
============================================
- Hits 31993 27215 -4778
+ Misses 37104 28281 -8823
+ Partials 3676 3018 -658
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests | `46.51% <ø> (+2.54%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pulsar/pull/18519?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...java/org/apache/pulsar/proxy/stats/TopicStats.java](https://codecov.io/gh/apache/pulsar/pull/18519/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLXByb3h5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9wdWxzYXIvcHJveHkvc3RhdHMvVG9waWNTdGF0cy5qYXZh) | `58.82% <0.00%> (-41.18%)` | :arrow_down: |
| [...g/apache/pulsar/broker/service/StreamingStats.java](https://codecov.io/gh/apache/pulsar/pull/18519/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL1N0cmVhbWluZ1N0YXRzLmphdmE=) | `75.67% <0.00%> (-8.11%)` | :arrow_down: |
| [.../org/apache/pulsar/broker/service/PulsarStats.java](https://codecov.io/gh/apache/pulsar/pull/18519/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL1B1bHNhclN0YXRzLmphdmE=) | `80.53% <0.00%> (-6.20%)` | :arrow_down: |
| [...ava/org/apache/pulsar/utils/StatsOutputStream.java](https://codecov.io/gh/apache/pulsar/pull/18519/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL3V0aWxzL1N0YXRzT3V0cHV0U3RyZWFtLmphdmE=) | `65.45% <0.00%> (-5.46%)` | :arrow_down: |
| [...he/pulsar/client/impl/PartitionedProducerImpl.java](https://codecov.io/gh/apache/pulsar/pull/18519/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1BhcnRpdGlvbmVkUHJvZHVjZXJJbXBsLmphdmE=) | `30.34% <0.00%> (-5.13%)` | :arrow_down: |
| [...rg/apache/pulsar/broker/namespace/OwnedBundle.java](https://codecov.io/gh/apache/pulsar/pull/18519/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9uYW1lc3BhY2UvT3duZWRCdW5kbGUuamF2YQ==) | `75.51% <0.00%> (-2.05%)` | :arrow_down: |
| [...a/org/apache/pulsar/proxy/server/ProxyService.java](https://codecov.io/gh/apache/pulsar/pull/18519/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLXByb3h5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9wdWxzYXIvcHJveHkvc2VydmVyL1Byb3h5U2VydmljZS5qYXZh) | `80.00% <0.00%> (-0.94%)` | :arrow_down: |
| [.../pulsar/client/impl/ProducerStatsRecorderImpl.java](https://codecov.io/gh/apache/pulsar/pull/18519/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1Byb2R1Y2VyU3RhdHNSZWNvcmRlckltcGwuamF2YQ==) | `84.04% <0.00%> (-0.62%)` | :arrow_down: |
| [...va/org/apache/pulsar/client/impl/ProducerImpl.java](https://codecov.io/gh/apache/pulsar/pull/18519/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1Byb2R1Y2VySW1wbC5qYXZh) | `16.43% <0.00%> (-0.59%)` | :arrow_down: |
| [...apache/pulsar/proxy/server/LookupProxyHandler.java](https://codecov.io/gh/apache/pulsar/pull/18519/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cHVsc2FyLXByb3h5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9wdWxzYXIvcHJveHkvc2VydmVyL0xvb2t1cFByb3h5SGFuZGxlci5qYXZh) | `57.75% <0.00%> (-0.44%)` | :arrow_down: |
| ... and [170 more](https://codecov.io/gh/apache/pulsar/pull/18519/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
--
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 #18519: [improve][client] Move acknowledge APIs to another interface and improve docs
Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on PR #18519:
URL: https://github.com/apache/pulsar/pull/18519#issuecomment-1319545411
@eolivelli Is there any more concern? If you concerned about the binary compatibility, from https://docs.oracle.com/javase/specs/jls/se7/html/jls-13.html, we can see
> here is a list of some important binary compatible changes that the Java programming language supports:
> ...
> Moving a method upward in the class hierarchy.
We can also have a simple verification like:
```
.
├── AImpl.java
├── v0
│ └── A.java
└── v1
├── A.java
└── F.java
```
where `AImpl` implements the interface `A`.
```
$ cat v0/A.java
public interface A {
void f();
void f(int x);
void g();
}
$ cat v1/A.java
public interface A extends F {
void g();
}
$ cat v1/F.java
public interface F {
void f(int x);
default void f() {
f(100);
}
}
$ javac AImpl.java v0/A.java
$ java -classpath $PWD/v0:$PWD AImpl
f()
f(1)
g()
$ javac v1/*.java
$ java -classpath $PWD/v1:$PWD AImpl
f()
f(1)
g()
```
--
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