exporters/vspec: warn on unmapped units instead of silent drop - #193
Open
SoundMatt wants to merge 1 commit into
Open
exporters/vspec: warn on unmapped units instead of silent drop#193SoundMatt wants to merge 1 commit into
SoundMatt wants to merge 1 commit into
Conversation
Previously, if a user wrote 'unit: SomeUnit = SOME_VALUE' in their schema and SOME_VALUE was not present in UNITS_DICT, the unit was dropped from the YAML output with no warning. The user intent silently vanished. Add a log.warning when an unmapped unit is encountered, naming the field and the unit value so the user can either add it to UNITS_DICT or adjust their schema. No behaviour change for mapped units. The existing TODO at this site is intentionally left in place because the broader 'use vss-tools dynamic_units directly' refactor is still outstanding. Adds tests/test_vspec.py covering both the warn-and-drop path and the no-warn path for mapped units. Signed-off-by: Matt Jones <47545907+SoundMatt@users.noreply.github.com>
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.
Small fix to a quiet data-loss path in the vspec exporter.
If a user writes
unit: SomeUnit = SOME_VALUEin their GraphQL schema andSOME_VALUEisn't present inUNITS_DICT(the hardcoded SCREAMING_SNAKE_CASE → abbreviated VSS name map at the top ofvspec.py), the unit was previously dropped from the YAML output with no warning — the user's intent silently vanished, and they'd have to diff the output to notice.This PR adds a
log.warningwhen an unmapped unit is encountered, naming both the field and the unit value, so the user can either:UNITS_DICT, orNo behaviour change for fields with mapped units.
The existing
TODOat this site about using vss-toolsdynamic_unitsdirectly is intentionally left in place — that's a bigger refactor and deserves its own PR.Tests
Adds
tests/test_vspec.pycovering:M→"m") and one unmapped unit (BANANA) → asserts the mapped one round-trips, the unmapped one is dropped, and the warning fires with both the field path and unit valueLocal verification: 534 passed (the deselected tests need the
graphql-inspectorNode CLI; CI has it).