Skip to content

KAFKA-10565: Only print console producer prompt with a tty#9644

Merged
chia7712 merged 1 commit intoapache:trunkfrom
tombentley:KAFKA-10565-console-producer-prompt
Nov 26, 2020
Merged

KAFKA-10565: Only print console producer prompt with a tty#9644
chia7712 merged 1 commit intoapache:trunkfrom
tombentley:KAFKA-10565-console-producer-prompt

Conversation

@tombentley
Copy link
Copy Markdown
Member

@tombentley tombentley commented Nov 23, 2020

This fixes https://issues.apache.org/jira/browse/KAFKA-10565 by using System.console to determine whether we're connected to a TTY before printing the kafka-console-producer.sh prompt. That's not a perfect solution (since stdin might be connected to the tty but not stdout, or vice versa), but it's still better that printing many prompts when reading from a pipe.

@tombentley
Copy link
Copy Markdown
Member Author

@chia7712 might you be able to review this? It's trivial, but I couldn't find a good way to test it except manually.

@chia7712
Copy link
Copy Markdown
Member

I couldn't find a good way to test it except manually.

@tombentley Could you attach the console screenshot ? I will test this PR manually later.

@tombentley
Copy link
Copy Markdown
Member Author

@chia7712 current behaviour:

Screenshot from 2020-11-24 10-40-59

With this patch:
Screenshot from 2020-11-24 10-45-06

@chia7712
Copy link
Copy Markdown
Member

@tombentley Thanks for your testing. Is it worth having a configuration to disable/enable to print prompt? The check of System.console is useful and the configuration make it more flexible. WDYT?

@tombentley
Copy link
Copy Markdown
Member Author

@chia7712 what's the use case for when you'd need to override the effects of using System.console? It would require a KIP, so adding a config is quite a lot more effort and I can't immediately see the benefit, but maybe you have something in mind.

Copy link
Copy Markdown
Member

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the use case for when you'd need to override the effects of using System.console? It would require a KIP, so adding a config is quite a lot more effort and I can't immediately see the benefit, but maybe you have something in mind.

oh, I have no use case for that. just a question :)

patch LGTM. Will commit it tomorrow if no objection.

@chia7712 chia7712 merged commit 6b1c8f9 into apache:trunk Nov 26, 2020
ijuma added a commit to ijuma/kafka that referenced this pull request Dec 2, 2020
…t-for-generated-requests

* apache-github/trunk: (405 commits)
KAFKA-6687: restrict DSL to allow only Streams from the same source
topics (apache#9609)
  MINOR: Small cleanups in `AlterIsr` handling logic (apache#9663)
MINOR: Increase unit test coverage of method
ProcessorTopology#updateSourceTopics() (apache#9654)
  MINOR: fix reading SSH output in Streams system tests (apache#9665)
  KAFKA-10770: Remove duplicate defination of Metrics#getTags (apache#9659)
  KAFKA-10722: Described the types of the used state stores (apache#9607)
  KAFKA-10702; Skip bookkeeping of empty transactions (apache#9632)
  MINOR: Remove erroneous extra <code> in design doc (apache#9657)
KAFKA-10736 Convert transaction coordinator metadata schemas to use g…
(apache#9611)
  MINOR: Update vagrant/tests readme (apache#9650)
  KAFKA-10720: Document prohibition on header mutation by SMTs (apache#9597)
  KAFKA-10713: Stricter protocol parsing in hostnames (apache#9593)
  KAFKA-10565: Only print console producer prompt with a tty (apache#9644)
  MINOR: fix listeners doc to close <code> properly (apache#9655)
  MINOR: Remove unnecessary statement from WorkerConnector#doRun (apache#9653)
KAFKA-10758: ProcessorTopology should only consider its own nodes when
updating regex source topics (apache#9648)
KAFKA-10754: fix flaky tests by waiting kafka streams be in running
state before assert (apache#9629)
  MINOR: Upgrade to Scala 2.13.4 (apache#9643)
  MINOR: Update build and test dependencies (apache#9645)
MINOR: Remove redundant argument from TaskMetricsGroup#recordCommit
(apache#9642)
  ...

clients/src/main/java/org/apache/kafka/clients/ClientRequest.java
clients/src/main/java/org/apache/kafka/clients/NetworkClient.java
clients/src/main/java/org/apache/kafka/common/requests/AbstractRequest.java
clients/src/main/java/org/apache/kafka/common/requests/AbstractRequestResponse.java
clients/src/main/java/org/apache/kafka/common/requests/AbstractResponse.java
clients/src/main/java/org/apache/kafka/common/requests/AlterConfigsResponse.java
clients/src/main/java/org/apache/kafka/common/requests/ApiVersionsResponse.java
clients/src/main/java/org/apache/kafka/common/requests/FetchRequest.java
clients/src/main/java/org/apache/kafka/common/requests/FetchResponse.java
clients/src/main/java/org/apache/kafka/common/requests/LeaderAndIsrRequest.java
clients/src/main/java/org/apache/kafka/common/requests/ListOffsetRequest.java
clients/src/main/java/org/apache/kafka/common/requests/ListOffsetResponse.java
clients/src/main/java/org/apache/kafka/common/requests/OffsetsForLeaderEpochRequest.java
clients/src/main/java/org/apache/kafka/common/requests/OffsetsForLeaderEpochResponse.java
clients/src/main/java/org/apache/kafka/common/requests/ProduceRequest.java
clients/src/main/java/org/apache/kafka/common/requests/ProduceResponse.java
clients/src/main/java/org/apache/kafka/common/requests/RequestHeader.java
clients/src/main/java/org/apache/kafka/common/requests/StopReplicaRequest.java
clients/src/main/java/org/apache/kafka/common/requests/UpdateMetadataRequest.java
clients/src/test/java/org/apache/kafka/clients/NetworkClientTest.java
clients/src/test/java/org/apache/kafka/clients/consumer/internals/FetcherTest.java
clients/src/test/java/org/apache/kafka/clients/producer/internals/SenderTest.java
clients/src/test/java/org/apache/kafka/common/requests/ProduceResponseTest.java
clients/src/test/java/org/apache/kafka/common/requests/RequestContextTest.java
clients/src/test/java/org/apache/kafka/common/requests/RequestHeaderTest.java
clients/src/test/java/org/apache/kafka/common/security/authenticator/SaslAuthenticatorTest.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants