shakeelansari63 commented on code in PR #26663:
URL: https://github.com/apache/superset/pull/26663#discussion_r1463162953
##########
helm/superset/templates/_helpers.tpl:
##########
@@ -61,6 +61,25 @@ Create chart name and version as used by the chart label.
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 |
trimSuffix "-" -}}
{{- end -}}
+{{/*
+Generate Redis URL for SSL and non-SSL protocols
+*/}}
+{{- define "superset-redis-url" -}}
+ {{- if .Values.supersetNode.connections.use_redis_ssl -}}
+ {{- if .Values.supersetNode.connections.redis_password -}}
+
rediss://:{env('REDIS_PASSWORD')}@{env('REDIS_HOST')}:{env('REDIS_PORT')}/##DBNAME##?ssl_cert_reqs={{-
.Values.supersetNode.connections.use_redis_ssl.ssl_cert_reqs | default
"CERT_NONE" -}}
+ {{- else -}}
+
rediss://{env('REDIS_HOST')}:{env('REDIS_PORT')}/##DBNAME##?ssl_cert_reqs={{-
.Values.supersetNode.connections.use_redis_ssl.ssl_cert_reqs | default
"CERT_NONE" -}}
Review Comment:
@ craig-rueda
Sure, will do that. Just 1 question. Previously we were using 2 redis
databases viz. `0` and `1`.
Under CeleryConfig, the database is set to `0`
```python
class CeleryConfig:
imports = ("superset.sql_lab", )
{{- if .Values.supersetNode.connections.redis_password }}
broker_url =
f"redis://:{env('REDIS_PASSWORD')}@{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
result_backend =
f"redis://:{env('REDIS_PASSWORD')}@{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
{{- else }}
broker_url = f"redis://{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
result_backend = f"redis://{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
{{- end }}
```
And Under Data Cache Config, database was set to `1`
As we don't have `REDIS_DB` envvar in current setup, the `env('REDIS_DB',
1)` will always return `1`
```python
CACHE_CONFIG = {
'CACHE_TYPE': 'RedisCache',
'CACHE_DEFAULT_TIMEOUT': 300,
'CACHE_KEY_PREFIX': 'superset_',
'CACHE_REDIS_HOST': env('REDIS_HOST'),
'CACHE_REDIS_PORT': env('REDIS_PORT'),
'CACHE_REDIS_PASSWORD': env('REDIS_PASSWORD'),
'CACHE_REDIS_DB': env('REDIS_DB', 1),
}
DATA_CACHE_CONFIG = CACHE_CONFIG
```
So how do you suggest we go with that?
1. Use 1 envvar `REDIS_DB` for both places and have it default value to `0`
2. Create 2 envvars `REDIS_DB` for DATA_CACHE_CONFIG with default `1` and
`REDIS_CELERY_DB` for CeleryConfig with default `0`
##########
helm/superset/templates/_helpers.tpl:
##########
@@ -61,6 +61,25 @@ Create chart name and version as used by the chart label.
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 |
trimSuffix "-" -}}
{{- end -}}
+{{/*
+Generate Redis URL for SSL and non-SSL protocols
+*/}}
+{{- define "superset-redis-url" -}}
+ {{- if .Values.supersetNode.connections.use_redis_ssl -}}
+ {{- if .Values.supersetNode.connections.redis_password -}}
+
rediss://:{env('REDIS_PASSWORD')}@{env('REDIS_HOST')}:{env('REDIS_PORT')}/##DBNAME##?ssl_cert_reqs={{-
.Values.supersetNode.connections.use_redis_ssl.ssl_cert_reqs | default
"CERT_NONE" -}}
+ {{- else -}}
+
rediss://{env('REDIS_HOST')}:{env('REDIS_PORT')}/##DBNAME##?ssl_cert_reqs={{-
.Values.supersetNode.connections.use_redis_ssl.ssl_cert_reqs | default
"CERT_NONE" -}}
Review Comment:
@craig-rueda
Sure, will do that. Just 1 question. Previously we were using 2 redis
databases viz. `0` and `1`.
Under CeleryConfig, the database is set to `0`
```python
class CeleryConfig:
imports = ("superset.sql_lab", )
{{- if .Values.supersetNode.connections.redis_password }}
broker_url =
f"redis://:{env('REDIS_PASSWORD')}@{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
result_backend =
f"redis://:{env('REDIS_PASSWORD')}@{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
{{- else }}
broker_url = f"redis://{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
result_backend = f"redis://{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
{{- end }}
```
And Under Data Cache Config, database was set to `1`
As we don't have `REDIS_DB` envvar in current setup, the `env('REDIS_DB',
1)` will always return `1`
```python
CACHE_CONFIG = {
'CACHE_TYPE': 'RedisCache',
'CACHE_DEFAULT_TIMEOUT': 300,
'CACHE_KEY_PREFIX': 'superset_',
'CACHE_REDIS_HOST': env('REDIS_HOST'),
'CACHE_REDIS_PORT': env('REDIS_PORT'),
'CACHE_REDIS_PASSWORD': env('REDIS_PASSWORD'),
'CACHE_REDIS_DB': env('REDIS_DB', 1),
}
DATA_CACHE_CONFIG = CACHE_CONFIG
```
So how do you suggest we go with that?
1. Use 1 envvar `REDIS_DB` for both places and have it default value to `0`
2. Create 2 envvars `REDIS_DB` for DATA_CACHE_CONFIG with default `1` and
`REDIS_CELERY_DB` for CeleryConfig with default `0`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
