You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "PaulRMellor (via GitHub)" <gi...@apache.org> on 2024/07/22 13:07:00 UTC
[PR] MINOR: docs(bootstrap.servers): updates the property description [kafka]
PaulRMellor opened a new pull request, #16655:
URL: https://github.com/apache/kafka/pull/16655
**Documentation**
Updates the `bootsrap.servers` property description for producer, consumer, connect, streams, and admin.
- Recommends listing more than one server
- States that the order does not matter as clients will first bootstrap using a random entry in the list and then automatically manage the connections as efficiently as possible
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: docs(bootstrap.servers): updates the property description [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison merged PR #16655:
URL: https://github.com/apache/kafka/pull/16655
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: docs(bootstrap.servers): updates the property description [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on code in PR #16655:
URL: https://github.com/apache/kafka/pull/16655#discussion_r1686579429
##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java:
##########
@@ -58,15 +58,12 @@ public class WorkerConfig extends AbstractConfig {
private static final Logger log = LoggerFactory.getLogger(WorkerConfig.class);
public static final String BOOTSTRAP_SERVERS_CONFIG = "bootstrap.servers";
- public static final String BOOTSTRAP_SERVERS_DOC
Review Comment:
Since it's the same doc than `CommonClientConfigs`, I think we could reuse `CommonClientConfigs.BOOTSTRAP_SERVERS_DOC` (and do the same for `BOOTSTRAP_SERVERS_CONFIG` above).
##########
clients/src/main/java/org/apache/kafka/clients/CommonClientConfigs.java:
##########
@@ -45,11 +45,11 @@ public class CommonClientConfigs {
*/
public static final String BOOTSTRAP_SERVERS_CONFIG = "bootstrap.servers";
- public static final String BOOTSTRAP_SERVERS_DOC = "A list of host/port pairs to use for establishing the initial connection to the Kafka cluster. The client will make use of all servers irrespective of which servers are specified here for bootstrapping—this list only impacts the initial hosts used to discover the full set of servers. This list should be in the form "
- + "<code>host1:port1,host2:port2,...</code>. Since these servers are just used for the initial connection to "
- + "discover the full cluster membership (which may change dynamically), this list need not contain the full set of "
- + "servers (you may want more than one, though, in case a server is down).";
Review Comment:
Is it really important to mention clients randomly pick an entry in the list when first bootstrapping?
##########
clients/src/main/java/org/apache/kafka/clients/CommonClientConfigs.java:
##########
@@ -45,11 +45,11 @@ public class CommonClientConfigs {
*/
public static final String BOOTSTRAP_SERVERS_CONFIG = "bootstrap.servers";
- public static final String BOOTSTRAP_SERVERS_DOC = "A list of host/port pairs to use for establishing the initial connection to the Kafka cluster. The client will make use of all servers irrespective of which servers are specified here for bootstrapping—this list only impacts the initial hosts used to discover the full set of servers. This list should be in the form "
- + "<code>host1:port1,host2:port2,...</code>. Since these servers are just used for the initial connection to "
- + "discover the full cluster membership (which may change dynamically), this list need not contain the full set of "
- + "servers (you may want more than one, though, in case a server is down).";
Review Comment:
I wonder if we could just change `you may want` to `we recommend using`
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: docs(bootstrap.servers): updates the property description [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on code in PR #16655:
URL: https://github.com/apache/kafka/pull/16655#discussion_r1694955755
##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java:
##########
@@ -58,15 +58,12 @@ public class WorkerConfig extends AbstractConfig {
private static final Logger log = LoggerFactory.getLogger(WorkerConfig.class);
public static final String BOOTSTRAP_SERVERS_CONFIG = "bootstrap.servers";
- public static final String BOOTSTRAP_SERVERS_DOC
Review Comment:
Yeah I decided to keep this change focused on the description change. We can do the refactoring in a follow up PR.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: docs(bootstrap.servers): updates the property description [kafka]
Posted by "PaulRMellor (via GitHub)" <gi...@apache.org>.
PaulRMellor commented on code in PR #16655:
URL: https://github.com/apache/kafka/pull/16655#discussion_r1686795294
##########
clients/src/main/java/org/apache/kafka/clients/CommonClientConfigs.java:
##########
@@ -45,11 +45,11 @@ public class CommonClientConfigs {
*/
public static final String BOOTSTRAP_SERVERS_CONFIG = "bootstrap.servers";
- public static final String BOOTSTRAP_SERVERS_DOC = "A list of host/port pairs to use for establishing the initial connection to the Kafka cluster. The client will make use of all servers irrespective of which servers are specified here for bootstrapping—this list only impacts the initial hosts used to discover the full set of servers. This list should be in the form "
- + "<code>host1:port1,host2:port2,...</code>. Since these servers are just used for the initial connection to "
- + "discover the full cluster membership (which may change dynamically), this list need not contain the full set of "
- + "servers (you may want more than one, though, in case a server is down).";
Review Comment:
I've updated and think the description reads well now, but let me know if I've interpreted your comments incorrectly.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: docs(bootstrap.servers): updates the property description [kafka]
Posted by "mimaison (via GitHub)" <gi...@apache.org>.
mimaison commented on code in PR #16655:
URL: https://github.com/apache/kafka/pull/16655#discussion_r1688203561
##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java:
##########
@@ -58,15 +58,12 @@ public class WorkerConfig extends AbstractConfig {
private static final Logger log = LoggerFactory.getLogger(WorkerConfig.class);
public static final String BOOTSTRAP_SERVERS_CONFIG = "bootstrap.servers";
- public static final String BOOTSTRAP_SERVERS_DOC
Review Comment:
Unresolving as you did not replace the doc string with `CommonClientConfigs.BOOTSTRAP_SERVERS_DOC`
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINOR: docs(bootstrap.servers): updates the property description [kafka]
Posted by "PaulRMellor (via GitHub)" <gi...@apache.org>.
PaulRMellor commented on code in PR #16655:
URL: https://github.com/apache/kafka/pull/16655#discussion_r1694952270
##########
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java:
##########
@@ -58,15 +58,12 @@ public class WorkerConfig extends AbstractConfig {
private static final Logger log = LoggerFactory.getLogger(WorkerConfig.class);
public static final String BOOTSTRAP_SERVERS_CONFIG = "bootstrap.servers";
- public static final String BOOTSTRAP_SERVERS_DOC
Review Comment:
@mimaison Ah, sorry. I misinterpreted the comment. I see it's merged, so assuming okay for the description change at least.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org