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/24 15:55:05 UTC
[GitHub] [pulsar] nicoloboschi opened a new pull request, #18615: [fix][cli] Pulsar shell: do not exit on user interrupt during commands
nicoloboschi opened a new pull request, #18615:
URL: https://github.com/apache/pulsar/pull/18615
### Motivation
In Pulsar shell if there's a command that is running (e.g client consume) and the user send a SIGINT (ctrl+c) the shell should not exit but it should only stop the current command.
### Modifications
* Added native signals to JLine terminal to explictly handle SIGINT and SIGQUIT
### Verifying this change
Run `client consume` without any message in the topic and hit ctrl+c.
- [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `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] codecov-commenter commented on pull request #18615: [fix][cli] Pulsar shell: do not exit on user interrupt during commands
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #18615:
URL: https://github.com/apache/pulsar/pull/18615#issuecomment-1327414477
# [Codecov](https://codecov.io/gh/apache/pulsar/pull/18615?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 [#18615](https://codecov.io/gh/apache/pulsar/pull/18615?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a4533e2) into [master](https://codecov.io/gh/apache/pulsar/commit/cd85a67bd3a880732678b0dfd5f7769ad90a90ef?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cd85a67) will **decrease** coverage by `12.43%`.
> The diff coverage is `9.09%`.
[](https://codecov.io/gh/apache/pulsar/pull/18615?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 #18615 +/- ##
=============================================
- Coverage 47.39% 34.96% -12.44%
+ Complexity 10479 6497 -3982
=============================================
Files 698 618 -80
Lines 68070 58577 -9493
Branches 7279 6094 -1185
=============================================
- Hits 32264 20483 -11781
- Misses 32228 35378 +3150
+ Partials 3578 2716 -862
```
| Flag | Coverage Δ | |
|---|---|---|
| unittests | `34.96% <9.09%> (-12.44%)` | :arrow_down: |
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/18615?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...pache/pulsar/broker/admin/v2/PersistentTopics.java](https://codecov.io/gh/apache/pulsar/pull/18615/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-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi92Mi9QZXJzaXN0ZW50VG9waWNzLmphdmE=) | `13.90% <ø> (-60.20%)` | :arrow_down: |
| [...apache/pulsar/broker/service/TopicListService.java](https://codecov.io/gh/apache/pulsar/pull/18615/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-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9zZXJ2aWNlL1RvcGljTGlzdFNlcnZpY2UuamF2YQ==) | `10.40% <0.00%> (-44.52%)` | :arrow_down: |
| [...pache/pulsar/client/impl/TableViewBuilderImpl.java](https://codecov.io/gh/apache/pulsar/pull/18615/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-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1RhYmxlVmlld0J1aWxkZXJJbXBsLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...a/org/apache/pulsar/client/impl/TableViewImpl.java](https://codecov.io/gh/apache/pulsar/pull/18615/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-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1RhYmxlVmlld0ltcGwuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...pulsar/client/impl/TableViewConfigurationData.java](https://codecov.io/gh/apache/pulsar/pull/18615/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-cHVsc2FyLWNsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1RhYmxlVmlld0NvbmZpZ3VyYXRpb25EYXRhLmphdmE=) | `33.33% <50.00%> (+33.33%)` | :arrow_up: |
| [...n/java/org/apache/pulsar/client/api/RawReader.java](https://codecov.io/gh/apache/pulsar/pull/18615/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-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9hcGkvUmF3UmVhZGVyLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pulsar/broker/admin/v1/Brokers.java](https://codecov.io/gh/apache/pulsar/pull/18615/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-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi92MS9Ccm9rZXJzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pulsar/broker/admin/v1/Clusters.java](https://codecov.io/gh/apache/pulsar/pull/18615/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-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi92MS9DbHVzdGVycy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../org/apache/pulsar/broker/admin/v1/Properties.java](https://codecov.io/gh/apache/pulsar/pull/18615/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-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2Jyb2tlci9hZG1pbi92MS9Qcm9wZXJ0aWVzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../org/apache/pulsar/client/impl/RawMessageImpl.java](https://codecov.io/gh/apache/pulsar/pull/18615/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-cHVsc2FyLWJyb2tlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9pbXBsL1Jhd01lc3NhZ2VJbXBsLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [288 more](https://codecov.io/gh/apache/pulsar/pull/18615/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] nicoloboschi commented on a diff in pull request #18615: [fix][cli] Pulsar shell: do not exit on user interrupt during commands
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on code in PR #18615:
URL: https://github.com/apache/pulsar/pull/18615#discussion_r1032236357
##########
pulsar-client-tools/src/main/java/org/apache/pulsar/shell/PulsarShell.java:
##########
@@ -217,7 +223,18 @@ public void reload(Properties properties) throws Exception {
}
public void run() throws Exception {
- final Terminal terminal = TerminalBuilder.builder().build();
+ final Terminal terminal = TerminalBuilder.builder()
+ .nativeSignals(true)
+ .signalHandler(signal -> {
+ if (signal == Terminal.Signal.INT || signal == Terminal.Signal.QUIT) {
+ if (execState == ExecState.RUNNING) {
+ throw new InterruptShellException();
Review Comment:
This class is mono-thread
If you double click control+c the shell exits because the first one makes the running command terminate and the second one exit the shell (as before)
the command termination one is handled here: https://github.com/apache/pulsar/blob/f8e632a45eece08d5c3454c6681484f02c0150cb/pulsar-client-tools/src/main/java/org/apache/pulsar/shell/PulsarShell.java#L453-L457
the shell termination is here:
https://github.com/apache/pulsar/blob/f8e632a45eece08d5c3454c6681484f02c0150cb/pulsar-client-tools/src/main/java/org/apache/pulsar/shell/PulsarShell.java#L424-L431
--
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 #18615: [fix][cli] Pulsar shell: do not exit on user interrupt during commands
Posted by GitBox <gi...@apache.org>.
tisonkun commented on PR #18615:
URL: https://github.com/apache/pulsar/pull/18615#issuecomment-1327361741
/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] tisonkun commented on a diff in pull request #18615: [fix][cli] Pulsar shell: do not exit on user interrupt during commands
Posted by GitBox <gi...@apache.org>.
tisonkun commented on code in PR #18615:
URL: https://github.com/apache/pulsar/pull/18615#discussion_r1032198072
##########
pulsar-client-tools/src/main/java/org/apache/pulsar/shell/PulsarShell.java:
##########
@@ -217,7 +223,18 @@ public void reload(Properties properties) throws Exception {
}
public void run() throws Exception {
- final Terminal terminal = TerminalBuilder.builder().build();
+ final Terminal terminal = TerminalBuilder.builder()
+ .nativeSignals(true)
+ .signalHandler(signal -> {
+ if (signal == Terminal.Signal.INT || signal == Terminal.Signal.QUIT) {
+ if (execState == ExecState.RUNNING) {
+ throw new InterruptShellException();
Review Comment:
In which thread is this exception thrown to?
Since we swallow `InterruptShellException` below, I'm unsure how the shell exits after the RUNNING command finished.
--
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] nicoloboschi merged pull request #18615: [fix][cli] Pulsar shell: do not exit on user interrupt during commands
Posted by GitBox <gi...@apache.org>.
nicoloboschi merged PR #18615:
URL: https://github.com/apache/pulsar/pull/18615
--
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