You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@curator.apache.org by "pantherdd (via GitHub)" <gi...@apache.org> on 2023/03/23 13:55:18 UTC
[GitHub] [curator] pantherdd opened a new pull request, #452: CURATOR-638: Use getHostString() to build connection string in EnsembleTracker even for wildcard addresses
pantherdd opened a new pull request, #452:
URL: https://github.com/apache/curator/pull/452
This is an additional fix based on PR #425.
--
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@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] kezhuw commented on a diff in pull request #452: CURATOR-638: Use getHostString() to build connection string in EnsembleTracker even for wildcard addresses
Posted by "kezhuw (via GitHub)" <gi...@apache.org>.
kezhuw commented on code in PR #452:
URL: https://github.com/apache/curator/pull/452#discussion_r1151305511
##########
curator-client/src/main/java/org/apache/curator/utils/Compatibility.java:
##########
@@ -88,7 +88,7 @@ public static boolean hasAddrField()
return (addrField != null);
}
- public static String getHostAddress(QuorumPeer.QuorumServer server)
+ public static String getHostString(QuorumPeer.QuorumServer server)
Review Comment:
I think it is ok for this renaming as `Compatibility` is not intended for external usage .
##########
curator-framework/src/test/java/org/apache/curator/framework/imps/TestReconfiguration.java:
##########
@@ -477,11 +477,35 @@ public void testConfigToConnectionStringNoClientAddrOrPort() throws Exception
}
@Test
- public void testHostname() throws Exception
+ public void testConfigToConnectionStringPreservesHostnameNormal() throws Exception
{
- String config = "server.1=localhost:2888:3888:participant;localhost:2181";
+ String config = "server.1=server.addr:2888:3888:participant;client.addr:2181";
Review Comment:
Thank you for correct this, it is my fault in creating this misunderstanding test.
--
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@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] kezhuw commented on pull request #452: CURATOR-638: Use getHostString() to build connection string in EnsembleTracker even for wildcard addresses
Posted by "kezhuw (via GitHub)" <gi...@apache.org>.
kezhuw commented on PR #452:
URL: https://github.com/apache/curator/pull/452#issuecomment-1486089186
Hi @pantherdd , any reason for this change ? [CURATOR-638](https://issues.apache.org/jira/browse/CURATOR-638) emphasizes preference for **configured** DNS name over resolved IP address which become outdated if quorum server migrated. For configured "0.0.0.0", I think "127.0.0.1" is more suitable than "localhost".
--
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@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] pantherdd commented on pull request #452: CURATOR-638: Use getHostString() to build connection string in EnsembleTracker even for wildcard addresses
Posted by "pantherdd (via GitHub)" <gi...@apache.org>.
pantherdd commented on PR #452:
URL: https://github.com/apache/curator/pull/452#issuecomment-1487624310
Hi @kezhuw ,
My intention was exactly that, to preserve the configured host name even if `clientAddr` is a wildcard address. Sorry for the potentially misleading test case, I just copied the one above and modified it slightly.
I updated both the code and the tests to clarify the behavior post-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@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] pantherdd commented on pull request #452: CURATOR-664: Use getHostString() to build connection string in EnsembleTracker even for wildcard addresses
Posted by "pantherdd (via GitHub)" <gi...@apache.org>.
pantherdd commented on PR #452:
URL: https://github.com/apache/curator/pull/452#issuecomment-1488465983
There seems to have been some transient network issue during the [PR workflow](https://github.com/apache/curator/actions/runs/4552169337/jobs/8028078609?pr=452):
> Failed to read artifact descriptor for `javax.activation:activation:jar:1.1.1`: Could not transfer artifact `javax.activation:activation:pom:1.1.1` from/to central ([https://repo.maven.apache.org/maven2](https://repo.maven.apache.org/maven2)): transfer failed for [https://repo.maven.apache.org/maven2/javax/activation/activation/1.1.1/activation-1.1.1.pom](https://repo.maven.apache.org/maven2/javax/activation/activation/1.1.1/activation-1.1.1.pom): Connect to `repo.maven.apache.org:443` [`repo.maven.apache.org/146.75.28.215`] failed: Connection timed out (Connection timed out) -> [Help 1]
--
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@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] eolivelli merged pull request #452: CURATOR-664: Use getHostString() to build connection string in EnsembleTracker even for wildcard addresses
Posted by "eolivelli (via GitHub)" <gi...@apache.org>.
eolivelli merged PR #452:
URL: https://github.com/apache/curator/pull/452
--
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@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [curator] pantherdd commented on pull request #452: CURATOR-664: Use getHostString() to build connection string in EnsembleTracker even for wildcard addresses
Posted by "pantherdd (via GitHub)" <gi...@apache.org>.
pantherdd commented on PR #452:
URL: https://github.com/apache/curator/pull/452#issuecomment-1488208243
> Nice work! Thank for your contribution. I have created a separate jira issue [CURATOR-664](https://issues.apache.org/jira/browse/CURATOR-664) for this. Would you mind reword commit message and pr title ?
Sure thing, done. Unfortunately I can't rename the source branch, but that's probably not very important anyway.
--
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@curator.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org