chore(feature_flags): warn on empty schema and empty rules - #8430
Conversation
SchemaValidator accepted an empty top-level document and any feature whose 'rules' key was present but falsy (including a list) without any signal. These are harmless for evaluation, but they usually indicate an authoring mistake such as a typo'd envelope path. Emit a warning log for an empty schema, for a feature whose 'rules' is present but empty, and a more specific warning when the empty value is not a dictionary. Nothing is raised, so existing documents keep validating. Omitting 'rules' entirely stays silent since that is the documented way to declare a static flag. Pass the real feature name into RulesValidator. It previously derived the name from the feature's first key (usually 'default'), so error and warning messages named the wrong thing. Closes #8427
powertools-for-aws-oss-automation
Bot
added
the
size/L
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #8430 +/- ##
========================================
Coverage 96.65% 96.66%
========================================
Files 296 296
Lines 14905 14914 +9
Branches 1266 1269 +3
========================================
+ Hits 14407 14416 +9
Misses 363 363
Partials 135 135 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Thanks Andrea. The validation changes and the feature name fix make sense. There is one difference between Python and TypeScript here. In Python, the standard way to emit a user-facing warning is I think this follows the TypeScript implementation, where using the logger makes more sense. I’m sending a small commit using |
|
2 participants
Issue number: closes #8427
Summary
Changes
SchemaValidatornow emits awarninglog for documents that are valid but almost certainly not what the author meant. Nothing is raised, so every document that validated before still validates.Three cases are covered:
{}). Typically the result of an envelope query that matched nothing, e.g. a typo'd feature group name.ruleskey is present but empty ({}orNone).ruleskey is present, empty, and not a dictionary ([],""). This gets a more specific message because the same value, once populated, would be rejected withSchemaValidationError; the empty form only slipped through becauseif not self.rulesshort-circuits first.Omitting
rulesentirely stays silent, since that is the documented way to declare a static flag.While wiring the warning I found that
RulesValidatorderivedfeature_namefromnext(iter(self.feature)), i.e. the feature's first key ("default"in practice), not its name.FeaturesValidatornow passes the real name in through a new optionalfeature_nameargument, so the new warnings and the existingFeature rules must be a dictionary, feature=...error name the right feature. The old fallback is kept so direct construction without the argument keeps working.Tests pin each warning's level and message, and pin the two silent paths (well-formed document,
rulesomitted) so the warnings do not become noisy.The TypeScript port (aws-powertools/powertools-lambda-typescript#5614) warns on the same cases and throws only where Python throws, so documents stay portable between runtimes.
User experience
No behaviour change for
evaluateorget_enabled_features. The only difference is in logs:{}WARNING Feature flags schema is empty, no features to validate{"f": {"default": false, "rules": {}}}WARNING Feature has 'rules' but it is empty, feature=f{"f": {"default": false, "rules": []}}WARNING Feature 'rules' should be a dictionary but is an empty list, feature=f{"f": {"default": false}}{"f": {"default": false, "rules": "4"}}feature=finstead offeature=defaultBy submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.