[ConfigManager] Register Sections 4/4 - #3977
Draft
bdchatham wants to merge 5 commits into
Draft
Conversation
Five sections whose keys belong to the Cosmos server, and the two registry capabilities they need. api 8 keys base 14 keys grpc 11 keys state-sync 3 keys telemetry 7 keys These sections have no owning package here. Their structs and their readers live in sei-cosmos, which this repository vendors rather than authors, so there is nowhere upstream to put a registration this registry would see. Four of the five register the upstream struct directly, because its mapstructure tags already name the keys the reader resolves. A section can now declare keys at the root of the file. The node-wide settings are written at the top of app.toml and read as pruning and halt-height, with no segment in front, so a section carrying a name into every key would rename all fourteen and an operator's existing file would reach none of them. A section therefore has a name it is looked up by and a prefix its keys carry, and for a root section the prefix is empty. Both walks build a key through one function, so a root key gains no separator on either side; reverting either one on its own fails a test, the value walk through the check that a rendered default states one value per declared key. Two keys can now collide where two prefixes never could. A key two sections both declare has one default rendered over the other, and which one depends on the order the sections are walked. And a root key that is also a section's name cannot be written at all, because a file holding both a value for that name and a table under it is not valid TOML, so one of the two is unreachable and nothing says which. Both are refused, in either registration order. A section can now say that an environment variable cannot supply one of its keys. The metric label set is a list of name and value rows and its reader asserts that exact shape rather than casting what it finds, so no single string satisfies it, and the assertion is the first statement of the whole server configuration. A resolved variable would install a value that stops the node; leaving the channel out means the file's value applies and the node runs. The reason is required rather than optional, because an operator whose variable is ignored has to be told why, and a refusal with no reason is itself refused. The metric section is the one here that needs a schema, and for one field's shape rather than for a spelling. Its label set is declared as untyped rows to match what the reader takes. A test holds every other field to the upstream field's name, tag and type, and holds the count of differing types at one, so a second divergence is a failure and a converged upstream type leaves the schema with nothing to justify it. Nothing here varies a default by mode. seid init writes the two interface toggles and the block retention per mode, so a node it provisioned carries those as written values, and these are what a node with nothing written runs. One declared value is not what a running node uses, and it is worth knowing which. The pruning strategy is declared as keeping everything, while the command line registers a flag of the same name defaulting to the standard strategy, and a bound flag is a source of its own below the file. A node started with no pruning key written prunes on the standard schedule. Whoever resolves for a running node has to supply the flag values to get the answer that node uses. The recorded configuration surface does not move, because nothing consumes the registry on a boot path yet.
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3977 +/- ##
==========================================
- Coverage 58.69% 57.62% -1.07%
==========================================
Files 2326 2230 -96
Lines 199122 187919 -11203
==========================================
- Hits 116877 108293 -8584
+ Misses 71477 69750 -1727
+ Partials 10768 9876 -892
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Three of these settings mean something different depending on what kind of node asks, and all five sections answered the same for every one. The binary already states the rules, in what it applies when it writes a file, and every section here now answers through them. Each of the three matters in a different direction. A full node and an archive node exist to serve queries, and both interfaces that serve them were declared closed. A validator is meant to expose as little as it can, and gRPC was declared open on every one of them, which is the opposite of what the rule beside it says it is for. And the number of blocks a node retains was declared as keeping everything for a full node, where the rule prunes at a hundred thousand. The rules are read rather than restated, so one added later moves these sections with nothing here changing, and the test writes the three values out by kind of node so a change to the rules fails and gets looked at. Resolving every mode as a validator, opening gRPC on a validator, and changing the retention each fail it. The two sections no rule touches answer through the same function, so there is one place a mode is applied rather than a decision per section about whether to apply it.
…d nothing A resolution answered for any string. A section's defaults answer per mode, and a mode this package does not know reached whatever each section does with an argument it cannot match, which for these five is the rules answering as though it were a full node. So an empty string, a capitalised name, or one with a trailing space resolved the interfaces a full node serves onto whichever node asked, with no error. It is refused now, naming the four. A refusal of the environment channel is recorded by a key, so a slip in the spelling named a key no section declares. The channel would never have offered that key, so the refusal covered nothing while reading as though it did, and the key it was written for went on resolving from a variable. Both sets exist for the first time when something resolves, because a refusal may be recorded before the section declaring its key registers, so that is where they are compared. The reason a refusal carries is required and had no consumer. The channel was skipped before the variable was read, so the one fact a diagnostic needs, that an operator set it, was discarded at the cheapest possible point. The variable is read now and its value still thrown away, and the key comes back named, so a required reason is one somebody can be told. A refused key nobody set is not reported, because a value nobody chose is not news. A refusal also names the section that declares the key, so a refused key is attributable the way every other defect is. It was putting the key where the section belongs, which made a defect read as though a key had registered and made a scoped sweep skip it. Four of the metric section's seven hand-copied values were held against nothing. That is the one section here that has to restate its values, so it is the one where a field can be assigned from its neighbour, and assigning the hostname toggle from the enabled toggle survived the suite. Every one of the seven is now held as the key it resolves to rather than as a struct field, because a struct compared with itself agrees while two values sit on the wrong fields. Five comments said things the code does not. The node-wide settings claimed to be unchanged by mode while one of their own keys answers per mode. A count of non-zero defaults was wrong. Two different counts of six read as one, and the pair the sentence lost is read through a clamp that does nothing for an absent key. The package's reason for existing named a vendored tree, when other sections register inside one and the real obstacle is an import edge. And a paragraph named two sections that belong to another change.
Four statements in the package contract described the previous shape. A second entry point exists, for the settings written at the top of a file rather than inside a table, and the contract showed one. The list of what makes a registration unusable no longer enumerated: two sections declaring one key and a top-level key sharing a section's name both became possible once a key could sit at the root, and a refusal of the environment carrying no reason is refused too. The resolution order had gained a per-key hole in one channel and did not say so. And the first step of adding a section told an author to use the name as the first segment of every key, which is false for a section whose keys have none. A mode this package does not declare is also refused now, and the contract says that where it says a default answers per mode.
refuseOverlap refused two collisions and only one of them could happen. Two sections declaring one key is already refused by the environment check, which two identical keys reach by answering to one variable, so that arm was a second guard on a case already covered. The other arm, a key at the top of the file sharing a section's name, was the only one it alone caught, and it has no instance: one section declares keys at the top of the file and none of its fourteen names is a section's. So the code goes and the fact stays. The contract names the collision among the things this package does not guard, with what makes it reachable, because a second such section is where it starts to matter. The prototype found that out by hand: it named the section holding config.toml's top-level keys after the file rather than after the node, because the client file declares a top-level key called node and a node section could not have coexisted with it. The one case the removed guard described better is named better now where it is still refused. Two identical keys were being reported as two spellings of one environment variable, and the reason a dot and a hyphen are the same character to the environment is not the reason a key collides with itself.
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
1 participant
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.
Registers the five configuration sections whose keys belong to the Cosmos server, and
adds the two registry capabilities they need.
These sections have no owning package here. Their structs and their readers live in
sei-cosmos, which this repository vendors rather than authors, so there is nowhereupstream to put a registration this registry would see. Four of the five register the
upstream struct directly, because its mapstructure tags already name the keys the reader
resolves.
A section can declare keys at the root of the file
The node-wide settings are written at the top of
app.tomland read aspruningandhalt-height, with no segment in front. A section that carried its name into every keywould rename all fourteen, and an operator's existing file would reach none of them. So
a section now has a name it is looked up and reported under, and a prefix its keys
carry, and for a root section the prefix is empty.
Both walks build a key through one function. Reverting either one on its own fails a
test: the type walk through the key comparison, and the value walk through the existing
check that a rendered default states one value per declared key.
One collision is refused, and one is a gap we are tracking
A key two sections both declare has one default rendered over the other, and which one
depends on the order the sections are walked. That is refused — two identical keys answer
to one environment variable, so the check that already refuses a spelling collision
catches it. It now says so in the right terms: a key colliding with itself is not two
spellings of one variable.
The gap. A key at the top of the file that is also a section's name cannot be written
at all, because no file holds both a value for that name and a table under it, so one of
the two settings is unreachable and nothing says which. Nothing refuses that.
I had a guard for it and removed it, because it has no instance: one section declares keys
at the top of the file and none of its fourteen names is a section's name. It becomes
reachable at the second such section, and the prototype shows that is exactly where it
bites — it named the section holding
config.toml's top-level keys after the file ratherthan after the node, because the client file declares a top-level key
nodeand anodesection could not have coexisted with it. So the guard belongs with the change that adds
the second root section, where the naming decision it forces actually arises.
The fact is recorded in the package contract, among the things the registry does not
guard, with the condition that makes it reachable. It is adjacent to PLT-1039, which
tracks the same shape of problem inside a single section: a leaf and a subtree declared
under one tag.
A section can say the environment cannot supply a key
The metric label set is a list of name and value rows, and its reader asserts that exact
shape rather than casting what it finds, so no single string satisfies it. That
assertion is the first statement of the whole server configuration, so a resolved
variable installs a value that stops the node. Leaving the channel out means the file's
value applies and the node runs. The reason is required rather than optional, because an
operator whose variable is ignored has to be told why, and a refusal with no reason is
itself refused.
The one schema, and what holds it
The metric section needs a schema, for one field's shape rather than for a spelling: its
label set is declared as untyped rows to match what the reader takes. A test holds every
other field to the upstream field's name, tag and type, and holds the count of differing
types at exactly one. A second divergence fails, and if the upstream type ever comes to
match, the schema is left with nothing to justify it and that fails too.
These five are declared and not yet linked
Nothing imports
config/cosmosbase, so itsinitruns only in its own test binary and these fivesections do not enter a real
seid. That is deliberate for now: adding a blank import somewhere in thecommand tree would put fourteen node-wide keys into the key space a node resolves, which is a behaviour
change and belongs with the work that consumes them rather than with the work that declares them.
Two consequences worth stating rather than leaving to be discovered. A diagnostic run against a real
binary will not see these keys. And the cross-section refusals — the two that depend on what else has
registered — are exercised against probes in the registry's own tests and never against the real
registered set, because that set and these five are never in one binary. Linking them is the first step
of whichever change consumes the registry.
One declared value is not what a running node resolves, and several more are not either
pruningis the one worth naming: the command line registers a flag of the same name defaulting to thestandard schedule, and a bound flag is a source below the file, so a node started with no pruning key
written prunes on that schedule while this declares it keeps everything. It is not the only one — a
command that assembles the server configuration overrides some of these before a node starts, and the
count is measured rather than asserted here.
A caller resolving for a running node therefore has to supply that node's flag values, and only the ones
an operator actually set. A flag nobody typed still reports a default, and this resolution ranks flags
above the file, so passing defaults would put every one of them over an operator's own value.
What I broke on purpose
Eight mutations, each caught: porting the key-joining into only one of the two walks
(both directions), rendering a root section's defaults under its lookup name, dropping
the collision refusal, dropping the environment skip, allowing a refusal with no reason,
resolving the label set as the shape its reader refuses, and giving the node-wide keys a
section prefix.
Two things to know
One declared value is not what a running node uses. The pruning strategy is declared
as keeping everything (
DefaultConfig()), while the command line registers a flag ofthe same name defaulting to the standard strategy, and a bound flag is a source of its
own below the file. A node started with no pruning key written prunes on the standard
schedule. Whoever resolves for a running node has to supply the flag values to get the
answer that node uses. Nothing depends on this yet; I can file it if it should be
tracked.
Three defaults vary by kind of node, and every section here answers through the mode
rules the binary already applies rather than restating them. A full node and an archive node
exist to serve queries, so both interfaces that serve them are on; a validator is meant to
expose as little as it can, so both are off, and the upstream default would have declared
gRPC open on every validator. Block retention follows the same rules. The test writes the
three values out by kind of node, so a change to the rules fails it and gets looked at.
The recorded configuration surface does not move, because nothing consumes the registry
on a boot path yet.
The manifest that states which sections a binary declares is deliberately not here. It
has to name every section, so it can only land once these four registration changes are
all on
main.