Omit offering tiers this environment cannot describe - #947
Merged
Conversation
/v1/offering advertised Large and XLarge in local dev as "Up to 0 subgraphs, 20 GB, 7-day retention". Billing defines what is purchasable and graph.yml defines the specs; where they disagree the tier was listed anyway, with max_subgraphs fabricated as 0 and the limits falling through to the Standard-shaped defaults in get_graph_limits / get_backup_limits. Price and credits looked right because those come from billing config, which has no environment dimension — which is what made the output plausible. Development defines only ladybug-standard, and correctly so: it runs a single graph_api, so Large and XLarge cannot exist there. The fix is not to add tier blocks that describe machines dev will never provision, but to stop answering a question the environment cannot answer. A tier with no config is now skipped. "Zero subgraphs, 20 GB" is a confident wrong answer where the truth is "not offered here". Omitting says that, and lets a client keep its own defaults rather than overwrite them with fabricated ones. Removes the now-dead `if tier_config` guards below the skip. The parity test is narrowed to match: where a tier IS defined outside production its subgraph count and storage cap must match, but absence is allowed. It also surfaced pre-existing drift — staging runs Large/XLarge at 2.5/5.0 api_rate_multiplier against production's 1.5/2.5 — left alone and excluded from the assertion, since looser staging limits are plausibly deliberate headroom rather than a mistake.
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
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.
/v1/offeringadvertised Large and XLarge in local dev as "Up to 0 subgraphs, 20 GB, 7-day retention" — plausible-looking numbers that were entirely fabricated.Why it looked believable
Billing defines what is purchasable;
graph.ymldefines the specs. Where they disagree, the tier was listed anyway:That asserts "this tier has zero subgraphs" when the truth is "I have no config for it." Storage and retention then fell through to the Standard-shaped defaults in
get_graph_limits/get_backup_limits.Price and credits were correct throughout, because those come from
billing/core.py, which has no environment dimension. That split — price right, every spec wrong — is what made the output convincing.The fix is subtraction, not addition
The obvious move was to add Large/XLarge blocks to
development. That would be fiction: dev runs a single graph_api, so those tiers cannot exist there, and the instance blocks would describe machines nothing will ever provision.Instead, a tier with no config for the environment is now skipped. "Zero subgraphs, 20 GB" is a confident wrong answer where the truth is "not offered here" — omitting says that, and lets a client keep its own defaults rather than overwrite them with fabricated ones.
No schema change, so no SDK regen. Production is unaffected: all three tiers have configs and all three still list.
Also removes the now-dead
if tier_configguards below the skip.Pre-existing drift, deliberately left alone
The parity test surfaced that staging runs Large/XLarge at
api_rate_multiplier2.5/5.0 against production's 1.5/2.5. Not touched and excluded from the assertion — looser staging limits are plausibly intentional headroom for load testing, and that's a product call rather than something to normalize silently.The test now allows a tier to be absent from an environment but requires that where it is present, its subgraph count and storage cap match production.
Testing
Full suite: 11,385 passed,
just test-codeclean. The only failures are the two pre-existingtest_incremental_materializecases that reproduce onmainand pass in isolation.Paired with robosystems-app, which guards the client side of the same bug —
??falls back on null/undefined but not0, so a fabricated zero overrode a correct fallback.