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

RK-19823 - add new DOP params #85

Closed
wants to merge 4 commits into from
Closed

Conversation

DavidRBiderman
Copy link

No description provided.

@DavidRBiderman
Copy link
Author

DavidRBiderman commented Sep 14, 2023

Github Enforcer opened Task: RK-19823

@sonariorobot sonariorobot changed the title add new DOP params RK-19823 - add new DOP params Sep 14, 2023
@@ -163,6 +163,9 @@ datastore:
# -- whether to deploy a Datastore
enabled: true

# -- whether to enable the new DAO wrappers for the datastore
enableNewDao: false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to add it to the readme

Copy link
Contributor

@gosharo gosharo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DavidRBiderman why do you change these chart? It is customer-facing and deprecated
https://github.com/Rookout/helm-charts#available-charts

@DavidRBiderman
Copy link
Author

@DavidRBiderman why do you change these chart? It is customer-facing and deprecated https://github.com/Rookout/helm-charts#available-charts
@gosharo
@nissim-infra asked to add it to the legacy chart in case someone still uses it and they upgrade the datastore

@@ -125,6 +125,7 @@ helm upgrade --install rookout rookout/rookout-hybrid \
| datastore.serviceAccount.create | bool | `true` | Specifies whether a service account should be created |
| datastore.serviceAccount.name | string | `""` | The name of the service account to use. If not set and create is true, a name is generated using the fullname template |
| datastore.tolerations | list | `[]` | [Tolerations] for use with node taints |
| datastore.enableNewDao | bool | `false` | Enable new SQL connector and new DAO schema
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we not also recommend increasing the cpu in that case?

Copy link
Author

@DavidRBiderman DavidRBiderman Sep 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ElDuderinos received all the disclaimers. It does not require nothing but it is recommended that the client increase resources generally speaking. This is not a mandatory step

@@ -125,6 +125,7 @@ helm upgrade --install rookout rookout/rookout-hybrid \
| datastore.serviceAccount.create | bool | `true` | Specifies whether a service account should be created |
| datastore.serviceAccount.name | string | `""` | The name of the service account to use. If not set and create is true, a name is generated using the fullname template |
| datastore.tolerations | list | `[]` | [Tolerations] for use with node taints |
| datastore.enableNewDao | bool | `false` | Enable new SQL connector and new DAO schema
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gosharo Should we bump to 0.3.2 (at the top)?

@gosharo gosharo closed this Sep 18, 2023
@gosharo
Copy link
Contributor

gosharo commented Sep 18, 2023

as @DavidRBiderman asked me to tag,
@dudiRookout @DavidRBiderman this is should be documented in docs.rookout.com.
Not relevent to charts.. it's applciation level configuration and not infra level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants