[charts][trino-gateway] add routing rules #295
Conversation
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
There was a problem hiding this comment.
I think we do NOT want to do this .. we are in the process of getting routing rules into the database and having a UI to modify them as well. Also we designed the chart to be as simple as possible since we are still changing configs around quite a bit.
As a result we have to carefully decide how to proceed .. I think this approach is too simple and doesnt take these other developments into account yet. I would not want to introduce this new config and then shortly after decide that we want to remove it again.
Thoughts @willmostly ?
As a result we have to carefully
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
|
Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla |
| * `replicaCount` - int, default: `1` | ||
| * `image.repository` - string, default: `"trinodb/trino-gateway"` |
There was a problem hiding this comment.
There are many changes in the README that do not appear to be related to the changes in your values.yaml. Please revert and then use precommit (see instructions in base README) to make sure the README is updated appropriately
There was a problem hiding this comment.
If you wish to move forward with this, please add test cases for
- basic case
- non-default rules path
- updated rules
As stated by @mosabua, we are moving away from file based rules in favor of storing them in the DB. Given that, and since file based rules can be implemented by mounting a configmap, supporting rules within the Helm chart may not be worth the additional complexity. Happy to discuss your use case however.
| @@ -31,6 +31,9 @@ imagePullSecrets: [] | |||
| envFrom: [] | |||
|
|
|||
| config: | |||
| routingRules: | |||
| rulesEngineEnabled: false | |||
| rulesConfigPath: "/opt/gateway/routing_rules/routing_rules.yml" | |||
There was a problem hiding this comment.
nit: use the yaml extension instead of yml
| @@ -31,6 +31,9 @@ imagePullSecrets: [] | |||
| envFrom: [] | |||
|
|
|||
| config: | |||
| routingRules: | |||
| rulesEngineEnabled: false | |||
| rulesConfigPath: "/opt/gateway/routing_rules/routing_rules.yml" | |||
There was a problem hiding this comment.
This change appears to be incomplete. The routing-rules configmap is not mounted anywhere
[charts][trino-gateway] add routing rules
@nineinchnick @mosabua can you please review? Thanks
cla sent today via email