github-actions[bot] commented on code in PR #64670:
URL: https://github.com/apache/doris/pull/64670#discussion_r3534079609
##########
fe/fe-core/src/main/java/org/apache/doris/common/proc/FrontendsProcNode.java:
##########
@@ -123,7 +124,7 @@ public static void getFrontendsInfo(Env env,
List<List<String>> infos, String cu
info.add(fe.getNodeName());
info.add(fe.getHost());
info.add(Integer.toString(fe.getEditLogPort()));
- info.add(Integer.toString(Config.http_port));
+ info.add(Integer.toString(HttpURLUtil.getHttpPort()));
Review Comment:
This changes the public `HttpPort` value returned by both `SHOW FRONTENDS`
and the `frontends()` TVF, but the PR does not add any coverage for the new
`enable_https=true` branch. Existing coverage only checks metadata/shape
(`ShowFrontendsCommandTest` and `test_frontends_tvf.groovy` do not set distinct
`http_port`/`https_port` values or assert this cell), while downstream helpers
consume this column as FE endpoint metadata through `getMasterPort("http")` /
`getFrontendIpHttpPort()`. Please add a focused test that toggles
`Config.enable_https` with different `Config.http_port` and `Config.https_port`
values and verifies this row reports the intended port, restoring the static
config afterward.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
