WIP: feat(status) display failed_reason for a failing Kafka instance#476
WIP: feat(status) display failed_reason for a failing Kafka instance#476rkpattnaik780 merged 2 commits intomainfrom
Conversation
| Name string `json:"name,omitempty"` | ||
| Status string `json:"status,omitempty"` | ||
| BootstrapServerHost string `json:"bootstrap_server_host,omitempty" title:"Bootstrap URL"` | ||
| FailedReason string `json:"failed_reason,omitempty" title:"Failed Reason"` |
There was a problem hiding this comment.
omitempty is not getting rid of the FailedReason key when the value is "". Same goes with other fields(tested by hardcoding).
There was a problem hiding this comment.
Okay, we will need to update the status printing logic in printServiceStatus to skip the field if omitempty is set and the value is empty.
| } | ||
| } else { | ||
| logger.Debug("No Kafka instance is currently used, skipping status check") | ||
| logger.Debug(localizer.MustLocalizeFromID("status.log.debug.noKafkaSelected")) |
There was a problem hiding this comment.
Shouild this be logger.Info instead? The user should be able to see this message irrespective of passing the --debug flag, what do you think @craicoverflow
There was a problem hiding this comment.
What convention should we follow to localize strings splitted like this?
logger.Info("Run", color.CodeSnippet("rhoas kafka use --id=<kafka-instance-id>"), "to use another Kafka instance.")
There was a problem hiding this comment.
I don't think they need to see that information all the time.
If I use rhoas for only one service "A", and there are 25 other services unused called "B" - "Z", you would get something like this:
$ rhoas status
A
-------------------
Blah:
Blah:
Blah:
Blah:
No B instance is currently used, skipping status check
No C instance is currently used, skipping status check
No D instance is currently used, skipping status check
No E instance is currently used, skipping status check
No F instance is currently used, skipping status check
No G instance is currently used, skipping status check
No H instance is currently used, skipping status check
No I instance is currently used, skipping status check
No J instance is currently used, skipping status check
No K instance is currently used, skipping status check
No L instance is currently used, skipping status check
No M instance is currently used, skipping status check
..."No B instance is currently used, skipping status check" does not tell the user much more than they can already figure out by there being no status information about the service in the output, but it is useful if they know that this service is currently used, they can check if it is being skipped, which can be useful to debug the problem.
There was a problem hiding this comment.
What convention should we follow to localize strings splitted like this?
logger.Info("Run", color.CodeSnippet("rhoas kafka use --id=<kafka-instance-id>"), "to use another Kafka instance.")
This was only split because color.CodeSnippet was used so it was easier to mix into the string. The alternative was to use log.Printf.
Since we are using localization now, there will be no need for splitting the strings like this as the full sentence is formed in the locales file.
2 participants
Description
Display
failed_reasonin status of Kafka instances that are failing.Fixes #474
Verification Steps
statuscommand to see the status.Type of change
Checklist