Refactor output colorization and improve heartbeat and listener commands - #1105
Merged
Conversation
"om daemon hb stop 1.rxx" exited zero and did nothing. The action is published as a message the heartbeat janitor subscribes to by component name, so a name no stream answers to was accepted, queued, and acted on by nobody. The name went through untouched: "9.rx", a stream of a heartbeat the cluster does not configure, was the same silence, and "om daemon hb sign 1.rx" read hb#1.rx.type, a key no configuration has. The listener actions were given that check when they were aligned with the audit subsystem names. The heartbeat ones have it now, from the same place and in the same shape: the name is checked against the heartbeats the cluster configuration defines, and the api answers 400 naming what it would have accepted. The two actions take two different things, which is what the name to refuse loudest was: start, stop and restart address one direction of a heartbeat, "1.rx", and sign and wipe address the heartbeat itself, "1". The "hb#" prefix the id column of "om daemon hb status" shows is accepted in both, so a name read there can be typed back rather than edited first. The commands say all of that now. Every heartbeat and listener action has a description of what NAME is and examples of it being used, the listener ones name the two listeners that exist, and the heartbeat ones, whose names are per cluster, complete from the local configuration: "om daemon hb start <tab>" offers the streams, "om daemon hb sign <tab>" the heartbeats. The examples render under the name the binary was invoked as, so an ox help does not tell its reader to type om. Checked on a live three node cluster: the refusals name the streams, "om daemon hb stop 4.rx" then "om daemon hb start hb#4.rx" stops and restarts the receiver, and the listener refusal is unchanged.
"om daemon listener restart api.ux" exited zero and logged, on the node it was asked of: listener: ux: daemon control restart asked, ignored api: ux: ask to restart component: api.ux The name is a listener, so the check the actions gained when they were aligned with the audit subsystem names let it through, and the api answered "daemon action queued". The listener then dropped it. That listener has no start, stop or restart of its own: it lives as long as the daemon does, and the request asking for its restart travels through it. The three actions used to be its log level actions, and when those were dropped the branch stayed, to log that it ignored what was left. So the api refuses them now, saying what it would have accepted, the way an unknown name is refused. The name check runs first: a name no listener answers to is not a listener with no lifecycle, it is no listener at all. The listener subscribes to no daemon control message any more. The message it was handed was the only one it could receive, and it existed to be ignored. The commands say it: the help of the three actions names api.inet as the value, and names api.ux too, with the reason it is not one, because a reader who knows it exists from an audit or a status would read its absence as an oversight. The completion offers what the api accepts. The api schema carries the same paragraph. Checked on a live cluster: the three actions on api.ux exit 1 with the reason, a restart of api.inet stops and starts it, and an unknown name is refused as before.
The two directions of a heartbeat run and stop on their own, which is why an action addresses one of them. They are one thing to a reader of "om daemon hb status", who saw hb#2 beating in both directions and had to type its two streams to stop it. "om daemon hb stop hb#2" answered that hb#2 was not a stream, listing the ten that are. A heartbeat named without a suffix now addresses both of its streams. The expansion is done where the name is checked, so it is the api that takes it and not just the command: the request publishes the message its name resolves to, one or two, and answers with what it queued. The actions take several names too. They ran one component each, and a reader stopping a heartbeat pair, or the same stream of two heartbeats, ran the command twice. Each name is attempted, and the failures are joined rather than the first ending the run: two unknown names are two errors, not one and a silence. The refusal names the heartbeats after the streams, so the name that prompted this reads as the shorthand it is rather than as unsupported. The completion offers both, and keeps offering after the first name, less the ones already typed. The help and the api schema say the two rules. Checked on a live cluster: "om daemon hb stop hb#2.tx 2.rx" stops the pair, whichever way each is spelled, "om daemon hb start hb#2" starts it back, and two unknown names exit 1 with both reasons.
"om daemon hb status" renders a row per stream, from a tab= column spec, filtered by --hb, read back by -o json. It is a listing. So are "om daemon relay status" and "om node relay status", from the same kind of spec. The tree has some forty listings and they are named list, so three of them answered to a word the rest do not, and a reader looking for the heartbeats had no way to guess which. The word is worth keeping, for the two commands that earn it. An instance status is a resource tree, where the nesting is the subsets and the resources. A cluster status is a board, where the nodes are the columns. Rendering either as rows would lose what the layout says. Neither is a listing, and neither should become one. So the rule is what the command renders, not what it is about: a row per thing is a list, aliased ls, and a composed view whose layout carries meaning is a status. A tab= column spec is the mechanical test, so the next command does not need the judgement call. It is written in CONTRIBUTING.md, which is the copy the repository tracks. The three are renamed. The name each answered to is built from the new one with Use overridden, the aliases cleared and Hidden set, as the deprecation policy asks and as the cluster status command already did: nothing that works today stops working, "status" only leaves the help and the completion. The help that pointed a reader at "om daemon hb status" to read a stream id points at "om daemon hb ls". Left alone: the cluster board still answers to om monitor, om ccfg status, a hidden om daemon status and om <kind> monitor. One view with four spellings is the larger duplication, and its own change.
"om <obj> config show --section fs#1" drew the "[fs#1]" line in italic when the section held a comment, and "om node config show" drew its "[DEFAULT]" line in italic when the file held one anywhere. The colorizer set every element with color.Set, which is the imperative form: it writes the escape sequence to the process output as a side effect and returns the color to render with. So each colored element left a bare, never reset sequence on stdout, and they all landed there ahead of the buffer the function returns: ^[[93;1m^[[36m^[[90m^[[90;3m^[[93;1m[DEFAULT]^[[0;22m nine of those, from one small config the line itself A comment leaks grey and italic. A section header opens with foreground and bold, which says nothing about italic, so the italic was still on when the header was drawn. Its terminator is a full reset, which is why only the first line was ever wrong, and why the section shown by --section is the one a reader notices. color.New builds the same color and writes nothing, which is what a function returning its rendering wants. The escape sequences are now the ones each element opens and closes for itself. One test swaps the package output for a buffer and reads it back empty, the other reads the first rendered line, with a comment inside the section, before it, and further down the file.
The heartbeat table entry composed the green "O" and the red "X" a listing draws its RUNNING and BEATING columns with. It is a daemon data type: it is published, an api client reads it, and it was reaching for an ansi library to hold escape sequences in two of the keys its Unstructured method returns. Nothing but "om daemon hb ls" ever read them. The tui builds the same two cells from the same values, e.State and e.IsBeating, and paints them with the markup its own widget takes, which is what the values are for. So the listing draws them. The row it renders embeds the entry and adds the two icon columns its default output names, from the state and the beating flag the entry carries. The output is unchanged, and the daemon tree imports no color library any more. One test sweeps the states, the beating flag and the single node case and reads every value of the entry back free of escape bytes, with the keys the icons are drawn from named, so removing one fails a test rather than emptying a column. The other reads the icons the listing composes, the single node among them: it beats with no peer, and is not stale for having none.
Two conventions the tree now follows, written down so they survive. color.Set writes its escape sequence to the process output as a side effect and returns the color to render with, so a function composing a string leaves a sequence on the terminal ahead of what it returns. That is what drew a commented section italic in "om <obj> config show". color.New builds the same color and writes nothing. And an escape sequence belongs to the code writing to the terminal, not to the type carrying the value: the heartbeat table entry used to hold the icons a listing draws, and it is published and read back by clients that paint their own. There is no color.Set call left to remove, so the first rule starts from a tree that keeps it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request introduces several updates aimed at improving how output is handled and enhancing the usability and clarity of certain commands. The key changes include:
color.Set. Instead,color.Newis used to ensure rendering is confined to the returned output, avoiding unexpected terminal behavior or color leaks.Tests