Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(feature_flags): support beyond boolean values (JSON values) #804

Merged
merged 14 commits into from
Dec 31, 2021

Conversation

ran-isenberg
Copy link
Contributor

@ran-isenberg ran-isenberg commented Nov 7, 2021

Issue #, if available: #777

Description of changes:

example schema

 app_config_schema = {
        "my_feature": {
            "default": [],
            "boolean_type": False,
            "rules": {
                "tenant id equals 345345435": {
                    "when_match": ["value1", "value2"],
                    "conditions": [
                        {
                            "action": "EQUALS",
                            "key": "tenant_id",
                            "value": "345345435",
                        }
                    ],
                }
            },
        }
    }

In this case, if you want to use non boolean types, you must supply boolean_type: False, otherwise True is assumed and a validation error is raised because "default" and when_match are not boolean.
If you dont write boolean_type at all, it's assumed as True and class acts as it did prior to this addition-> non breaking change for current users.

UX

feature_value: List[str] = features.evaluate(name="my_feature", context={"tenant_id": "345345435"}, default=[])
feature_value is ["value1", "value2"] because tenant_id = 345345435 and it matches the conditions.
feature_value: List[str] = features.evaluate(name="my_feature", context={"tenant_id": "1"}, default=[])
feature_value is [] because tenant_id = 1 and it  does NOT match the conditions.

Checklist

Breaking change checklist

RFC issue #:

  • Migration process documented
  • Implement warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 7, 2021
@codecov-commenter
Copy link

codecov-commenter commented Nov 7, 2021

Codecov Report

Merging #804 (c5bde49) into develop (f985c40) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #804   +/-   ##
========================================
  Coverage    99.96%   99.96%           
========================================
  Files          118      118           
  Lines         5273     5281    +8     
  Branches       606      606           
========================================
+ Hits          5271     5279    +8     
  Partials         2        2           
Impacted Files Coverage Δ
aws_lambda_powertools/shared/types.py 100.00% <100.00%> (ø)
...owertools/utilities/feature_flags/feature_flags.py 100.00% <100.00%> (ø)
...ambda_powertools/utilities/feature_flags/schema.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f985c40...c5bde49. Read the comment docs.

@heitorlessa heitorlessa added the p3 label Dec 8, 2021
@heitorlessa
Copy link
Contributor

Quick note - As we discussed, I'm happy with the direction. Here's what I'd like to have to better understand to craft a messaging and docs to better communicate this to customers.

  • Provide an example scenario along with schema and return values
  • Clarify what the return values should be - Are we returning the value it matches?
  • Add return type primitives instead of ANY

@heitorlessa heitorlessa changed the title feat: enhance feature flags to a generic engine with non boolean values feat(feature_flags): support beyond boolean values Dec 9, 2021
@ran-isenberg
Copy link
Contributor Author

@heitorlessa updated description with schema example and usage

@heitorlessa heitorlessa self-assigned this Dec 9, 2021
@heitorlessa
Copy link
Contributor

Thanks a lot @ran-isenberg - As we discussed, I'll work on fixing some bug reports on Parser for the next imminent release, then I'll get back to this PR for having a deeper look so we can find a way to communicate this feature better for customers.

If you could update the diagram, the mentions on the doc that is no longer a boolean only, and a concrete use case we could use to communicate the value that would help prioritize it for the 1.23.0 release (the one after next).

@michaelbrewer
Copy link
Contributor

@ran-isenberg @heitorlessa - i see we are missing a test for not boolean_feature cases where we skip over?

@ran-isenberg
Copy link
Contributor Author

@ran-isenberg @heitorlessa - i see we are missing a test for not boolean_feature cases where we skip over?

Didnt push all tests, waited for an approval for the idea. I'm working on it ;)

@boring-cyborg boring-cyborg bot added area/utilities documentation Improvements or additions to documentation labels Dec 9, 2021
@ran-isenberg
Copy link
Contributor Author

added a bunch of tests

@ran-isenberg
Copy link
Contributor Author

ran-isenberg commented Dec 9, 2021

@heitorlessa
potential uses case:
user permitted actions: for admins users you return a list of actions ['read', 'write']
but default users, feature toggle value is ['read']

 app_config_schema = {
        "my_feature": {
            "default": ['read'],
            "boolean_type": False,
            "rules": {
                "username startswith with admin": {
                    "when_match": ["read", "write"],
                    "conditions": [
                        {
                            "action":"STARTSWITH",
                            "key": "user",
                            "value": "admin",
                        }
                    ],
                }
            },
        }
    }

Another example can be for customer A allow a set of features/ configuration but provide customer B with just the basic.
Maybe filter the types of features by customer type. partner/beta tester type can get an expanded list of features.

 router_app_config_schema = {
        "my_feature": {
            "default": "when_match": ['a', 'b', 'c'], 
            "boolean_type": False,
            "rules": {
                "customer tier is premium": {
                    "when_match":  ['a', 'b', 'c', 'beta_feature'], 
                    "conditions": [
                        {
                            "action":"EQUALS",
                            "key": "customer_type",
                            "value": "beta_tester",
                        }
                    ],
                }
            },
        }
    }

Third example: routing tenants by tier. Save http url or configuration in the flag and route them by tier.
ARN is an ARN of an api gw/step function etc that is triggered according to tier type.

 router_app_config_schema = {
        "my_feature": {
            "default": {'arn': 'default arn noisy neighbor', 'allowed_region': ['us-east1']}, 
            "boolean_type": False,
            "rules": {
                "customer tier is premium": {
                    "when_match": {'arn': 'premium account arn', 'allowed_region': ['us-east1', 'us-west1']}, 
                    "conditions": [
                        {
                            "action":"EQUALS",
                            "key": "tier",
                            "value": "premium",
                        }
                    ],
                }
            },
        }
    }

@heitorlessa heitorlessa added p1 and removed p3 labels Dec 20, 2021
@heitorlessa
Copy link
Contributor

Looking to merge this one today

@ran-isenberg
Copy link
Contributor Author

@heitorlessa can't wait :)

@heitorlessa
Copy link
Contributor

The docs PR on consistency for all pages took way more hours than anticipated, I'm making a minor change on this tomorrow and merging before release (also tomorrow). Apologies for the delay

#919

@heitorlessa heitorlessa added the feature New feature or functionality label Dec 30, 2021
@heitorlessa
Copy link
Contributor

@ran-isenberg as we're in beta, does it worth making a note that this feature will be the default once/right before GA?

As in, it'd be great to not require this additional field in their schema -- return whatever the value when match or default value.

Keen to hear your thoughts as I know you're using in prod regardless of the label beta.

@ran-isenberg
Copy link
Contributor Author

I dont mind keeping the new field because that way we do get the boolean schema validation instead of allowing blindly everything. There's also the get all enabled flags API which I think should return only the boolean rules that match (it returns a list of string - name of features that are True).

@heitorlessa
Copy link
Contributor

heitorlessa commented Dec 31, 2021 via email

@ran-isenberg
Copy link
Contributor Author

ran-isenberg commented Dec 31, 2021

you mean a function that returns a List[Dict[ str, Any]] with feature name and the evaluated value?
that could work too

@heitorlessa
Copy link
Contributor

heitorlessa commented Dec 31, 2021 via email

…tools-python into complex

* 'develop' of https://github.com/awslabs/aws-lambda-powertools-python: (24 commits)
  docs: consistency around admonitions and snippets (aws-powertools#919)
  chore(deps-dev): bump mypy from 0.920 to 0.930 (aws-powertools#925)
  fix(event-sources): handle dynamodb null type as none, not bool (aws-powertools#929)
  fix(apigateway): support @app.not_found() syntax & housekeeping (aws-powertools#926)
  docs: Added GraphQL Sample API to Examples section of README.md (aws-powertools#930)
  feat(idempotency): support dataclasses & pydantic models payloads (aws-powertools#908)
  feat(tracer): ignore tracing for certain hostname(s) or url(s) (aws-powertools#910)
  feat(event-sources): cache parsed json in data class (aws-powertools#909)
  fix(warning): future distutils deprecation (aws-powertools#921)
  docs(batch): remove leftover from legacy
  docs(layer): bump Lambda Layer to version 6
  chore: bump to 1.23.0
  docs(apigateway): add new not_found feature (aws-powertools#915)
  docs: external reference to cloudformation custom resource helper (aws-powertools#914)
  feat(logger): allow handler with custom kwargs signature (aws-powertools#913)
  chore: minor housekeeping before release (aws-powertools#912)
  chore(deps-dev): bump mypy from 0.910 to 0.920 (aws-powertools#903)
  feat(batch): new BatchProcessor for SQS, DynamoDB, Kinesis (aws-powertools#886)
  fix(parser): overload parse when using envelope (aws-powertools#885)
  fix(parser): kinesis sequence number is str, not int (aws-powertools#907)
  ...
@pull-request-size pull-request-size bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 31, 2021
@heitorlessa heitorlessa changed the title feat(feature_flags): support beyond boolean values feat(feature_flags): support beyond boolean values (JSON values) Dec 31, 2021
@heitorlessa
Copy link
Contributor

Thanks again Ran for implementing this feature!

I've made the following changes to ease maintenance, plus docs. I'm merging, working on one last PR then publishing 1.24 release with it ;-)

  • Bugfix:
    • Allow non-boolean values to be returned upon rule match
    • 5 Mypy issues on types
    • Replaced Any with JSONType to ease Mypy and IDE findings for non-JSON primitives
  • Tests:
    • Merged tests into a single file, removed one/two duplicate
    • Renamed tests to be more explicit to the behaviour they're testing
    • Tests for truthy default non-boolean default values
  • Docs:
    • Updated Schema Validation API docs
    • Updated Schema section
    • New section Beyond boolean feature flags

image
image
image

@heitorlessa heitorlessa merged commit be15e3c into aws-powertools:develop Dec 31, 2021
heitorlessa added a commit to huonw/aws-lambda-powertools-python that referenced this pull request Dec 31, 2021
…tools-python into feature/905-datetime

* 'develop' of https://github.com/awslabs/aws-lambda-powertools-python:
  feat(feature_flags): support beyond boolean values (JSON values) (aws-powertools#804)
  docs: consistency around admonitions and snippets (aws-powertools#919)
  chore(deps-dev): bump mypy from 0.920 to 0.930 (aws-powertools#925)
  fix(event-sources): handle dynamodb null type as none, not bool (aws-powertools#929)
  fix(apigateway): support @app.not_found() syntax & housekeeping (aws-powertools#926)
  docs: Added GraphQL Sample API to Examples section of README.md (aws-powertools#930)
  feat(idempotency): support dataclasses & pydantic models payloads (aws-powertools#908)
  feat(tracer): ignore tracing for certain hostname(s) or url(s) (aws-powertools#910)
  feat(event-sources): cache parsed json in data class (aws-powertools#909)
  fix(warning): future distutils deprecation (aws-powertools#921)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation feature New feature or functionality p1 size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants