NextbrickInc commented on code in PR #4818:
URL: https://github.com/apache/solr/pull/4818#discussion_r3870763366
##########
solr/webapp/web/js/angular/controllers/core-overview.js:
##########
@@ -56,14 +56,16 @@ function($scope, $rootScope, $routeParams, Luke, CoreInfo,
Update, Replication,
$scope.refreshPing = function() {
Ping.status({core: $routeParams.core}, function(data) {
- if (data.error) {
+ if (data.status == "not_configured") {
$scope.healthcheckStatus = false;
- if (data.error.code == 503) {
- $scope.healthcheckMessage = 'Ping request handler is not configured
with a healthcheck file.';
- }
+ $scope.healthcheckMessage = 'Ping request handler is not configured
with a healthcheck file.';
} else {
+ delete $scope.healthcheckMessage;
$scope.healthcheckStatus = data.status == "enabled";
Review Comment:
No, and deliberately so — `"disabled"` is the state this widget was built
for, and it has to keep falling through to the toggle.
`isPingDisabled()` is `healthcheck != null && !Files.exists(healthcheck)`
(`PingRequestHandler.java:168`), so `"disabled"` means *a healthcheckFile is
configured and currently absent* — "ping is off, and there is a file I can
create to turn it back on". The template renders exactly that: with
`healthcheckMessage` unset and `healthcheckStatus` false,
`core_overview.html:192` shows the clickable **"enable ping"** control, and
`toggleHealthcheck()` succeeds because a file is configured.
`"not_configured"` is the mirror image, and that is why it cannot share the
branch: with no `healthcheckFile` at all, `handleEnable()` throws
`SERVICE_UNAVAILABLE — "No healthcheck file defined."`
(`PingRequestHandler.java:296-298`). Routing it to the message branch hides the
toggle (`ng-show="!healthcheckMessage"`), so the UI stops offering a button
that cannot work.
Your question did catch a real gap, though: any *other* status value — a
future one, or a typo — landed in the `else` and rendered the unlit "enable
ping" control, which is the one outcome we do not want for an unknown state.
Fixed in 9b5545ddb4 by naming the three known states:
```js
delete $scope.healthcheckMessage;
if (data.status == "enabled" || data.status == "disabled") {
$scope.healthcheckStatus = data.status == "enabled";
} else {
$scope.healthcheckStatus = false;
$scope.healthcheckMessage = data.status == "not_configured"
? 'Ping request handler is not configured with a healthcheck file.'
: 'Unexpected ping status: ' + data.status;
}
```
`enabled` / `disabled` / `not_configured` behave exactly as before; only the
unknown case moves.
--
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]
