-
Notifications
You must be signed in to change notification settings - Fork 412
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
Conversation
Codecov Report
@@ 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
Continue to review full report at Codecov.
|
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.
|
@heitorlessa updated description with schema example and usage |
…wertools-python into complex
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). |
@ran-isenberg @heitorlessa - i see we are missing a test for |
Didnt push all tests, waited for an approval for the idea. I'm working on it ;) |
added a bunch of tests |
@heitorlessa 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. 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. 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",
}
],
}
},
}
} |
Looking to merge this one today |
@heitorlessa can't wait :) |
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 |
@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. |
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). |
Gotcha - for the get all enabled features I was thinking in keeping it as
is, and create a new get all matching features to cater for the new returns.
As I want to release it today, I’ll mark to revisit before we go GA - my
only concern is the complexity of newcomers into the feature, not so much
about maintenance as we can make some minor changes.
I’ll go ahead and make the minimal changes to ease maintenance, help Mypy
inform issues, write the docs, and add a note to revisit this feature on
whether we can make it simpler
…On Fri, 31 Dec 2021 at 05:34, Ran Isenberg ***@***.***> wrote:
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).
—
Reply to this email directly, view it on GitHub
<#804 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZPQBBV6ASJS4BNDIU52Z3UTUXFPANCNFSM5HQ37NQQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
you mean a function that returns a List[Dict[ str, Any]] with feature name and the evaluated value? |
Yeah, just like we did for Parameters when you wanna get multiple
parameters at once (recursively for SSM, or as a Query for the same pk in
DynamoDB)
…On Fri, 31 Dec 2021 at 07:38, Ran Isenberg ***@***.***> wrote:
you mean a function that returns a Dict[ str, Any] with feature name and
the evaluated value?
that could work too
—
Reply to this email directly, view it on GitHub
<#804 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZPQBE44Z6GZZ6CD5RRUBTUTVFXRANCNFSM5HQ37NQQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
…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) ...
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 ;-)
|
…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)
Issue #, if available: #777
Description of changes:
example schema
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
Checklist
Breaking change checklist
RFC issue #:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.