You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pulsar.apache.org by ma...@gmail.com on 2023/01/30 08:15:58 UTC
[DISCUSS][txn] Move checked exception into builder when newTransaction.
Hello, everyone
I submitted this PR https://github.com/apache/pulsar/pull/19356 to discuss if we can accept moving this checked exception into the builder to avoid adding more useless try-catch blocks.
In practice, we should create the new transaction like this:
```java
final TransactionBuilder transactionBuilder;
try {
transactionBuilder = client.newTransaction();
} catch (PulsarClientException e) {
// handle this exception.
return;
}
CompletableFuture<Transaction> txnFuture = transactionBuilder
.withTransactionTimeout(1, TimeUnit.MINUTES)
.build();
```
But this exception only throws by
```java
public TransactionBuilder newTransaction() throws PulsarClientException {
if (!conf.isEnableTransaction()) {
throw new PulsarClientException.InvalidConfigurationException("Transactions are not enabled");
}
return new TransactionBuilderImpl(this, tcClient);
}
```
Therefore, even if we enabled the transaction, we still need to handle this checked exception. Maybe we can move this checked exception into the builder or make this exception to be runtime to avoid writing the more useless try-catch blocks.
```java
client.newTransaction()
.withTransactionTimeout(1, TimeUnit.MINUTES)
.build()
.thenCompose(transaction -> {
// do somethings
}).exceptionally(ex -> {
// handle exception
});
```
It is better, isn't it?
Best,
Mattison
Re: [DISCUSS][txn] Move checked exception into builder when newTransaction.
Posted by Nicolò Boschi <bo...@gmail.com>.
+1
Thanks,
Nicolò Boschi
Il giorno lun 30 gen 2023 alle ore 09:33 Zike Yang <zi...@apache.org> ha
scritto:
> Thanks for your PR. I think this looks good to me.
>
> Thanks,
> Zike Yang
>
> On Mon, Jan 30, 2023 at 4:23 PM Enrico Olivelli <eo...@gmail.com>
> wrote:
> >
> > I totally agree.
> > There has been little API Design error to add that "throws" clause.
> >
> > Thanks for the initiative
> >
> > Enrico
> >
> > Il giorno lun 30 gen 2023 alle ore 09:16 <ma...@gmail.com> ha
> scritto:
> > >
> > > Hello, everyone
> > >
> > > I submitted this PR https://github.com/apache/pulsar/pull/19356 to
> discuss if we can accept moving this checked exception into the builder to
> avoid adding more useless try-catch blocks.
> > >
> > > In practice, we should create the new transaction like this:
> > >
> > > ```java
> > > final TransactionBuilder transactionBuilder;
> > > try {
> > > transactionBuilder = client.newTransaction();
> > > } catch (PulsarClientException e) {
> > > // handle this exception.
> > > return;
> > > }
> > > CompletableFuture<Transaction> txnFuture = transactionBuilder
> > > .withTransactionTimeout(1, TimeUnit.MINUTES)
> > > .build();
> > > ```
> > > But this exception only throws by
> > >
> > > ```java
> > > public TransactionBuilder newTransaction() throws
> PulsarClientException {
> > > if (!conf.isEnableTransaction()) {
> > > throw new
> PulsarClientException.InvalidConfigurationException("Transactions are not
> enabled");
> > > }
> > > return new TransactionBuilderImpl(this, tcClient);
> > > }
> > > ```
> > > Therefore, even if we enabled the transaction, we still need to handle
> this checked exception. Maybe we can move this checked exception into the
> builder or make this exception to be runtime to avoid writing the more
> useless try-catch blocks.
> > >
> > > ```java
> > > client.newTransaction()
> > > .withTransactionTimeout(1, TimeUnit.MINUTES)
> > > .build()
> > > .thenCompose(transaction -> {
> > > // do somethings
> > > }).exceptionally(ex -> {
> > > // handle exception
> > > });
> > > ```
> > > It is better, isn't it?
> > >
> > > Best,
> > > Mattison
>
Re: [DISCUSS][txn] Move checked exception into builder when newTransaction.
Posted by Zike Yang <zi...@apache.org>.
Thanks for your PR. I think this looks good to me.
Thanks,
Zike Yang
On Mon, Jan 30, 2023 at 4:23 PM Enrico Olivelli <eo...@gmail.com> wrote:
>
> I totally agree.
> There has been little API Design error to add that "throws" clause.
>
> Thanks for the initiative
>
> Enrico
>
> Il giorno lun 30 gen 2023 alle ore 09:16 <ma...@gmail.com> ha scritto:
> >
> > Hello, everyone
> >
> > I submitted this PR https://github.com/apache/pulsar/pull/19356 to discuss if we can accept moving this checked exception into the builder to avoid adding more useless try-catch blocks.
> >
> > In practice, we should create the new transaction like this:
> >
> > ```java
> > final TransactionBuilder transactionBuilder;
> > try {
> > transactionBuilder = client.newTransaction();
> > } catch (PulsarClientException e) {
> > // handle this exception.
> > return;
> > }
> > CompletableFuture<Transaction> txnFuture = transactionBuilder
> > .withTransactionTimeout(1, TimeUnit.MINUTES)
> > .build();
> > ```
> > But this exception only throws by
> >
> > ```java
> > public TransactionBuilder newTransaction() throws PulsarClientException {
> > if (!conf.isEnableTransaction()) {
> > throw new PulsarClientException.InvalidConfigurationException("Transactions are not enabled");
> > }
> > return new TransactionBuilderImpl(this, tcClient);
> > }
> > ```
> > Therefore, even if we enabled the transaction, we still need to handle this checked exception. Maybe we can move this checked exception into the builder or make this exception to be runtime to avoid writing the more useless try-catch blocks.
> >
> > ```java
> > client.newTransaction()
> > .withTransactionTimeout(1, TimeUnit.MINUTES)
> > .build()
> > .thenCompose(transaction -> {
> > // do somethings
> > }).exceptionally(ex -> {
> > // handle exception
> > });
> > ```
> > It is better, isn't it?
> >
> > Best,
> > Mattison
Re: [DISCUSS][txn] Move checked exception into builder when newTransaction.
Posted by Enrico Olivelli <eo...@gmail.com>.
I totally agree.
There has been little API Design error to add that "throws" clause.
Thanks for the initiative
Enrico
Il giorno lun 30 gen 2023 alle ore 09:16 <ma...@gmail.com> ha scritto:
>
> Hello, everyone
>
> I submitted this PR https://github.com/apache/pulsar/pull/19356 to discuss if we can accept moving this checked exception into the builder to avoid adding more useless try-catch blocks.
>
> In practice, we should create the new transaction like this:
>
> ```java
> final TransactionBuilder transactionBuilder;
> try {
> transactionBuilder = client.newTransaction();
> } catch (PulsarClientException e) {
> // handle this exception.
> return;
> }
> CompletableFuture<Transaction> txnFuture = transactionBuilder
> .withTransactionTimeout(1, TimeUnit.MINUTES)
> .build();
> ```
> But this exception only throws by
>
> ```java
> public TransactionBuilder newTransaction() throws PulsarClientException {
> if (!conf.isEnableTransaction()) {
> throw new PulsarClientException.InvalidConfigurationException("Transactions are not enabled");
> }
> return new TransactionBuilderImpl(this, tcClient);
> }
> ```
> Therefore, even if we enabled the transaction, we still need to handle this checked exception. Maybe we can move this checked exception into the builder or make this exception to be runtime to avoid writing the more useless try-catch blocks.
>
> ```java
> client.newTransaction()
> .withTransactionTimeout(1, TimeUnit.MINUTES)
> .build()
> .thenCompose(transaction -> {
> // do somethings
> }).exceptionally(ex -> {
> // handle exception
> });
> ```
> It is better, isn't it?
>
> Best,
> Mattison