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/09/13 04:03:39 UTC
[GitHub] [pulsar] TakaHiR07 opened a new pull request, #17374: [fix][broker]fix catching ConflictException when update topic partition
TakaHiR07 opened a new pull request, #17374:
URL: https://github.com/apache/pulsar/pull/17374
### Motivation
A fix for #17251. There is an error when handling ConflictException.
The previous unittest can not approve it, so I add another test.
### Modifications
ex.getCause() -> ex
add test.
### Verifying this change
- [x] Make sure that the change passes the CI checks.
### 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`
(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] AnonHxy commented on pull request #17374: [fix][broker]fix catching ConflictException when update topic partition
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on PR #17374:
URL: https://github.com/apache/pulsar/pull/17374#issuecomment-1244906169
@Jason918 PTAL
--
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] HQebupt commented on pull request #17374: fix catching ConflictException when update topic partition
Posted by GitBox <gi...@apache.org>.
HQebupt commented on PR #17374:
URL: https://github.com/apache/pulsar/pull/17374#issuecomment-1232914573
Good catch! Please follow the PR naming standard. https://docs.google.com/document/d/1d8Pw6ZbWk-_pCKdOmdvx9rnhPiyuxwq60_TrD68d7BA/edit#heading=h.wu6ygjne8e35
--
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] TakaHiR07 commented on pull request #17374: [fix][broker]fix catching ConflictException when update topic partition
Posted by GitBox <gi...@apache.org>.
TakaHiR07 commented on PR #17374:
URL: https://github.com/apache/pulsar/pull/17374#issuecomment-1233662899
> Nice catch.
>
> The code below hide the exception, that's why the test can pass without throw exception. I think the code below wants to try to solve the same problem as #17251, but has some problem.
>
> I think that we can also remove the code from Line4399-Line4402. Because #17251 fix it better. WDYT @TakaHiR07
>
> https://github.com/apache/pulsar/blob/dab0d1f389ee66277c8350072af304895619eb9a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java#L4398-L4405
> Nice catch.
>
> The code below hide the exception, that's why the test can pass without throw exception. I think the code below wants to try to solve the same problem as #17251, but has some problem.
>
> I think that we can also remove the code from Line4399-Line4402. Because #17251 fix it better. WDYT @TakaHiR07
>
> https://github.com/apache/pulsar/blob/dab0d1f389ee66277c8350072af304895619eb9a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java#L4398-L4405
@AnonHxy I think it is good! We do not need this code after #17251.
I have force-push to contain this change.
--
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] AnonHxy commented on pull request #17374: [fix][broker]fix catching ConflictException when update topic partition
Posted by GitBox <gi...@apache.org>.
AnonHxy commented on PR #17374:
URL: https://github.com/apache/pulsar/pull/17374#issuecomment-1232943532
Nice catch.
The code below hide the exception, that's why the test can pass without throw exception. I think the code below wants to try to solve the same problem as https://github.com/apache/pulsar/pull/17251, but has some problem.
I think that we can also remove the code from Line4399-Line4402. Because https://github.com/apache/pulsar/pull/17251 fix it better. WDYT @TakaHiR07
https://github.com/apache/pulsar/blob/dab0d1f389ee66277c8350072af304895619eb9a/pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java#L4398-L4405
--
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] AnonHxy closed pull request #17374: [fix][broker]fix catching ConflictException when update topic partition
Posted by GitBox <gi...@apache.org>.
AnonHxy closed pull request #17374: [fix][broker]fix catching ConflictException when update topic partition
URL: https://github.com/apache/pulsar/pull/17374
--
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] TakaHiR07 commented on pull request #17374: fix catching ConflictException when update topic partition
Posted by GitBox <gi...@apache.org>.
TakaHiR07 commented on PR #17374:
URL: https://github.com/apache/pulsar/pull/17374#issuecomment-1232851214
@HQebupt @Jason918 @AnonHxy Please take a look.
--
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] AnonHxy merged pull request #17374: [fix][broker]fix catching ConflictException when update topic partition
Posted by GitBox <gi...@apache.org>.
AnonHxy merged PR #17374:
URL: https://github.com/apache/pulsar/pull/17374
--
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