-
Notifications
You must be signed in to change notification settings - Fork 94
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
[COST-1847] unleash client for trino enablement #3163
Conversation
/retest |
Codecov Report
@@ Coverage Diff @@
## main #3163 +/- ##
=======================================
- Coverage 94.0% 94.0% -0.0%
=======================================
Files 308 308
Lines 26172 26170 -2
Branches 2975 2974 -1
=======================================
- Hits 24596 24592 -4
- Misses 997 1000 +3
+ Partials 579 578 -1 |
84a26f8
to
63c30aa
Compare
/retest |
/retest |
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with a suggestion on formatting the JSON file
.unleash/flags.json
Outdated
@@ -0,0 +1 @@ | |||
{"version":1,"features":[{"name":"cost-trino-processor","description":"Toggle to enable trino processing","type":"permission","project":"default","enabled":true,"stale":false,"strategies":[{"name":"schema-strategy","parameters":{"schema-name":""},"constraints":[]}],"variants":[],"createdAt":"2021-09-14T21:22:00.756Z"}],"strategies":[{"name":"schema-strategy","description":"Enablement based on account/schema number","parameters":[{"name":"schema-name","type":"list","description":"values must begin with `acct`","required":false}]}]} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{"version":1,"features":[{"name":"cost-trino-processor","description":"Toggle to enable trino processing","type":"permission","project":"default","enabled":true,"stale":false,"strategies":[{"name":"schema-strategy","parameters":{"schema-name":""},"constraints":[]}],"variants":[],"createdAt":"2021-09-14T21:22:00.756Z"}],"strategies":[{"name":"schema-strategy","description":"Enablement based on account/schema number","parameters":[{"name":"schema-name","type":"list","description":"values must begin with `acct`","required":false}]}]} | |
{ | |
"version": 1, | |
"features": [ | |
{ | |
"name": "cost-trino-processor", | |
"description": "Toggle to enable trino processing", | |
"type": "permission", | |
"project": "default", | |
"enabled": true, | |
"stale": false, | |
"strategies": [ | |
{ | |
"name": "schema-strategy", | |
"parameters": { | |
"schema-name": "" | |
}, | |
"constraints": [] | |
} | |
], | |
"variants": [], | |
"createdAt": "2021-09-14T21:22:00.756Z" | |
} | |
], | |
"strategies": [ | |
{ | |
"name": "schema-strategy", | |
"description": "Enablement based on account/schema number", | |
"parameters": [ | |
{ | |
"name": "schema-name", | |
"type": "list", | |
"description": "values must begin with `acct`", | |
"required": false | |
} | |
] | |
} | |
] | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main reason why I didn't format this file is that we won't modify it directly. We should be exporting the flags from the unleash DB using make unleash-export
. But! I updated that make command to format the file on export, so it should be readable now.
/retest |
This PR adds the unleash client and allows us to utilize Unleash to enable trino processing for account/schema numbers. All the other mechanisms in place to enable trino processing will still work, so any
ENABLE_TRINO_SOURCES
,ENABLE_TRINO_SOURCE_TYPE
, orENABLE_TRINO_ACCOUNTS
that are already defined will function the same way.Adding the unleash client here allows us to add new accounts without the hassle of updating the deploy.yaml in app-interface.
Testing:
(this is easier to see without trino running)
acct10001
to the schema-name list in the schema-strategy for thecost-trino-processor
feature flagSmokes:
Manually ran the 2 failures: