Skip to content

chore(feature_flags): warn on empty schema and empty rules - #8430

Open
dreamorosi wants to merge 1 commit into
developfrom
chore/warn-on-odd-feature-flag-schemas
Open

chore(feature_flags): warn on empty schema and empty rules#8430
dreamorosi wants to merge 1 commit into
developfrom
chore/warn-on-odd-feature-flag-schemas

Conversation

@dreamorosi

Copy link
Copy Markdown
Contributor

Issue number: closes #8427

Summary

Changes

SchemaValidator now emits a warning log 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:

  • Empty top-level document ({}). Typically the result of an envelope query that matched nothing, e.g. a typo'd feature group name.
  • A feature whose rules key is present but empty ({} or None).
  • A feature whose rules key is present, empty, and not a dictionary ([], ""). This gets a more specific message because the same value, once populated, would be rejected with SchemaValidationError; the empty form only slipped through because if not self.rules short-circuits first.

Omitting rules entirely stays silent, since that is the documented way to declare a static flag.

While wiring the warning I found that RulesValidator derived feature_name from next(iter(self.feature)), i.e. the feature's first key ("default" in practice), not its name. FeaturesValidator now passes the real name in through a new optional feature_name argument, so the new warnings and the existing Feature 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, rules omitted) 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 evaluate or get_enabled_features. The only difference is in logs:

Document Before After
{} silent WARNING Feature flags schema is empty, no features to validate
{"f": {"default": false, "rules": {}}} silent WARNING Feature has 'rules' but it is empty, feature=f
{"f": {"default": false, "rules": []}} silent WARNING Feature 'rules' should be a dictionary but is an empty list, feature=f
{"f": {"default": false}} silent silent
{"f": {"default": false, "rules": "4"}} raises raises, now with feature=f instead of feature=default

By 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.

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 powertools-for-aws-oss-automation Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 3, 2026
@sonarqubecloud

sonarqubecloud Bot commented Sep 3, 2026

Copy link
Copy Markdown

@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.65%. Comparing base (a39e101) to head (c4cd833).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #8430   +/-   ##
========================================
  Coverage    96.65%   96.65%           
========================================
  Files          296      296           
  Lines        14767    14774    +7     
  Branches      1246     1249    +3     
========================================
+ Hits         14273    14280    +7     
  Misses         359      359           
  Partials       135      135           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dreamorosi
dreamorosi marked this pull request as ready for review September 3, 2026 16:38
@dreamorosi
dreamorosi requested a review from a team as a code owner September 3, 2026 16:38
@dreamorosi
dreamorosi requested review from leandrodamascena and svozza and removed request for svozza September 3, 2026 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tech debt: Feature Flags schema accepts odd-but-harmless documents silently

1 participant