You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/10/13 22:20:03 UTC
[GitHub] [superset] Yann-J opened a new pull request, #21806: feat(helm): Support for flower and websocket containers
Yann-J opened a new pull request, #21806:
URL: https://github.com/apache/superset/pull/21806
### SUMMARY
This PR brings a number of updates to the Helm chart:
- Support running an optional `flower` sidecar container in the celery beat pod, to visualize the tasks (but NOT exposed through the ingress, since most users probably don't want to expose this publicly - either a new ingress needs to be deployed, or a port-forward to the flower service is needed)
- Support the optional deployment of a `superset-websocket` server in a separate pod, together with associated services and ingress path within the same host(s)
- Reworked the probes to make them fully customizable through the `values.yaml`, added `startupProbes`, as well as a liveness probe for the celery workers
- Added an auto-generated `README`, using [helm-docs](https://github.com/norwoodj/helm-docs), to document the contents of `values.yaml` (and should be visible from artifacthub)
The more controversial change is also that I've removed the schema definition... I'm sorry but I've really find it too hard to work with, and was making iterative developments of the chart quite painfully slow, while not necessarily helping catch too many issues...
I guess this is debatable, and if that's a problem I can work on restoring it, but I personally suggest to drop it since I think it creates unnecessary work - unless there's a tool to generate it that I'm not aware of?
### TESTING INSTRUCTIONS
- Deploy a new release of the chart. To enable the new features:
- set `supersetCeleryBeat.flower.enabled=true` to see the flower sidecar container in the celerybeat pod, and access the UI by port-forwarding its port 5555. WARNING this requires a custom Superset image that has `flower<1.0.0` installed (recent versions are causing a dependency conflict)
- set `supersetWebsockets.enabled=true` to see the superset-websocket pod (by default a custom image I've built from our fork, since there's no official one). It assumes that both pods are exposed through an ingress on the same domain (the built-in ingress will do this automatically) and requires various things:
- `JWT_SECRET` set as a secret env variable - it will be passed to both Superset and websocket servers
- in `superset_config.py`:
- `GLOBAL_ASYNC_QUERIES_TRANSPORT=ws`
- A proper value for `GLOBAL_ASYNC_QUERIES_WEBSOCKET_URL` (I use `f"{re.sub('^http','ws',env('BASE_URL'))}/ws"`)
- `GLOBAL_ASYNC_QUERIES_JWT_SECRET=env("JWT_SECRET")`
This was tested successfully on Superset 1.5.2
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Yann-J commented on a diff in pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
Yann-J commented on code in PR #21806:
URL: https://github.com/apache/superset/pull/21806#discussion_r1003130423
##########
helm/superset/values.yaml:
##########
@@ -267,20 +267,18 @@ supersetNode:
# -- If true, forces deployment to reload on each upgrade
forceReload: false
# -- Init containers
- # @default -- a container waiting for postgres to be up
+ # @default -- a container waiting for postgres
initContainers:
- name: wait-for-postgres
- image: "{{ .Values.initImage.repository }}:{{ .Values.initImage.tag }}"
- imagePullPolicy: "{{ .Values.initImage.pullPolicy }}"
+ image: jwilder/dockerize
Review Comment:
Yeah, this is a good catch, and I probably should have mentioned it. I've hesitated on the best approach here.
The reason I'm not using the provided values, is to avoid risks of regressions, if, for some reason, someone has overridden these keys, the `dockerize` command will fail.
I don't think there's really any strong reason to be overriding these, but it's possible...
There might be a more elegant way to manage this potentially breaking change...
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Yann-J commented on a diff in pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
Yann-J commented on code in PR #21806:
URL: https://github.com/apache/superset/pull/21806#discussion_r1004270987
##########
helm/superset/values.yaml:
##########
@@ -267,20 +267,18 @@ supersetNode:
# -- If true, forces deployment to reload on each upgrade
forceReload: false
# -- Init containers
- # @default -- a container waiting for postgres to be up
+ # @default -- a container waiting for postgres
initContainers:
- name: wait-for-postgres
- image: "{{ .Values.initImage.repository }}:{{ .Values.initImage.tag }}"
- imagePullPolicy: "{{ .Values.initImage.pullPolicy }}"
+ image: jwilder/dockerize
Review Comment:
To reduce a bit the redundancy, we can use `initImage.repository` as a default, but still leave the possibility to independently override it separately for each deployment...
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Yann-J commented on pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
Yann-J commented on PR #21806:
URL: https://github.com/apache/superset/pull/21806#issuecomment-1293244416
Yes, I've seen those CI failures, and assumed this was some kind of known problem, since this is unlikely to be related to any of the changes in this PR.
I don't know very well how the CI was originally set up, but I can see that the `superset-helm-lint.yaml` workflow is set up to run `chart-testing`, however it doesn't specify any chart repos to install.
I think the fix would be to pass the list of dependent repos to install prior to running `chart-testing`. The `ct.yaml` configuration basically needs to include the following config (or passed as a `CT_CHART_REPOS` env var):
```
chart-repos:
- bitnami=https://charts.bitnami.com/bitnami
```
I'm sending some commits to try a fix, but it might take a while since this might needs some trial-and-error...
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Yann-J commented on pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
Yann-J commented on PR #21806:
URL: https://github.com/apache/superset/pull/21806#issuecomment-1290312587
I would still suggest to add `helm-docs` as a pre-commit hook - of course CI needs double-check it, but the local hook is a very useful reminder to run it before pushing, saving CI time to catch such a trivial issue...
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] craig-rueda commented on pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
craig-rueda commented on PR #21806:
URL: https://github.com/apache/superset/pull/21806#issuecomment-1294004733
LGTM, Thanks!
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] craig-rueda commented on a diff in pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
craig-rueda commented on code in PR #21806:
URL: https://github.com/apache/superset/pull/21806#discussion_r1003619173
##########
helm/superset/values.yaml:
##########
@@ -267,20 +267,18 @@ supersetNode:
# -- If true, forces deployment to reload on each upgrade
forceReload: false
# -- Init containers
- # @default -- a container waiting for postgres to be up
+ # @default -- a container waiting for postgres
initContainers:
- name: wait-for-postgres
- image: "{{ .Values.initImage.repository }}:{{ .Values.initImage.tag }}"
- imagePullPolicy: "{{ .Values.initImage.pullPolicy }}"
+ image: jwilder/dockerize
Review Comment:
I think it should be configurable in this case. the pull policy also :). We can just bump the chart version to indicate there's a potentially "breaking" change.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] craig-rueda commented on a diff in pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
craig-rueda commented on code in PR #21806:
URL: https://github.com/apache/superset/pull/21806#discussion_r997168713
##########
helm/superset/values.yaml:
##########
@@ -344,20 +342,18 @@ supersetWorker:
# -- If true, forces deployment to reload on each upgrade
forceReload: false
# -- Init container
- # @default -- a container waiting for postgres
+ # @default -- a container waiting for postgres and redis
initContainers:
- - name: wait-for-postgres
- image: "{{ .Values.initImage.repository }}:{{ .Values.initImage.tag }}"
- imagePullPolicy: "{{ .Values.initImage.pullPolicy }}"
+ - name: wait-for-postgres-redis
Review Comment:
Same comment as above RE values.yaml
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] craig-rueda commented on pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
craig-rueda commented on PR #21806:
URL: https://github.com/apache/superset/pull/21806#issuecomment-1279213867
> Yes, it will show tasks for all the workers, regardless of where it runs, because it connects to the broker. I just wanted to keep things simple, since this server is unlikely to ever require any scaling (and since celery beat has to be a singleton, I thought it was good enough). A separate pod would also work of course but was just more boilerplate...
Ok, understood. Can we pls make it another deployment? 🙏 This pod can scale depending on usage quite a bit (especially when running reports or generating thumbnails, for example)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] craig-rueda commented on pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
craig-rueda commented on PR #21806:
URL: https://github.com/apache/superset/pull/21806#issuecomment-1280994903
Sounds interesting, however pre-commit probably isn't the best spot, as folks can pretty easily skip them. What would be better would be a lint step in CI that checks the README against a generated version.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] craig-rueda merged pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
craig-rueda merged PR #21806:
URL: https://github.com/apache/superset/pull/21806
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Yann-J commented on pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
Yann-J commented on PR #21806:
URL: https://github.com/apache/superset/pull/21806#issuecomment-1290310806
OK, I've been convinced and switched back to using the shared defaults from `initImage.*` for the init containers. They can still be overridden separately for each `Deployment` if required.
There's a very low risk of regression, but it will presumably be very quick to identify and fix, for anyone who has overridden the previous `busybox` default...
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Yann-J commented on pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
Yann-J commented on PR #21806:
URL: https://github.com/apache/superset/pull/21806#issuecomment-1279674783
OK, this is done, flower is now in its own separate Deployment.
One more change I propose is to switch the `initContainers` that wait for dependencies from a shell script wait to [dockerize](https://github.com/jwilder/dockerize) which is more robust and versatile (e.g. can wait for multiple dependencies, in our case both postgres and redis).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Yann-J commented on pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
Yann-J commented on PR #21806:
URL: https://github.com/apache/superset/pull/21806#issuecomment-1293342714
OK, we now have a passing linting and chart-testing! Just pushed one final polish to tweak some docs, and I think we're finally all good!
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] craig-rueda commented on pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
craig-rueda commented on PR #21806:
URL: https://github.com/apache/superset/pull/21806#issuecomment-1292268794
Looks like there's some issues resolving bitnami?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Yann-J commented on a diff in pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
Yann-J commented on code in PR #21806:
URL: https://github.com/apache/superset/pull/21806#discussion_r1004268097
##########
helm/superset/values.yaml:
##########
@@ -267,20 +267,18 @@ supersetNode:
# -- If true, forces deployment to reload on each upgrade
forceReload: false
# -- Init containers
- # @default -- a container waiting for postgres to be up
+ # @default -- a container waiting for postgres
initContainers:
- name: wait-for-postgres
- image: "{{ .Values.initImage.repository }}:{{ .Values.initImage.tag }}"
- imagePullPolicy: "{{ .Values.initImage.pullPolicy }}"
+ image: jwilder/dockerize
Review Comment:
It is already all configurable, as the entire `initContainers` section is provided in the `values.yaml`. The default can be overridden directly. It's just that it won't be using `initImage.repository`, but rather `supersetNode.initContainers[0].image`.
The version will definitely be bumped, as I think chart versions should be immutable. I believe the chart release process will ensure this anyway.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Yann-J commented on pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
Yann-J commented on PR #21806:
URL: https://github.com/apache/superset/pull/21806#issuecomment-1279707532
Another suggestion (not implemented here) is to add `helm-docs` as a pre-commit hooks to make sure the chart's `README` is always up-to-date with the `values.yaml`.
Not sure if that's OK...
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Yann-J commented on pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
Yann-J commented on PR #21806:
URL: https://github.com/apache/superset/pull/21806#issuecomment-1279193785
Yes, it will show tasks for all the workers, regardless of where it runs, because it connects to the broker.
I just wanted to keep things simple, since this server is unlikely to ever require any scaling (and since celery beat has to be a singleton, I thought it was good enough). A separate pod would also work of course but was just more boilerplate...
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Yann-J commented on pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
Yann-J commented on PR #21806:
URL: https://github.com/apache/superset/pull/21806#issuecomment-1279665363
Yes, sure, I can do this if you think it's useful.
I'm not sure about the need for scaling, since (if I understand Celery correctly), the celery _workers_ will need scaling, but the celery _beat_ acts as a controller and needs to be a singleton.
Flower, being a management UI, is also unlikely to require any scaling.
That beind said, there's no harm in having it as a separate deployment, that's probably the _right_ way of doing this indeed...
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] craig-rueda commented on a diff in pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
craig-rueda commented on code in PR #21806:
URL: https://github.com/apache/superset/pull/21806#discussion_r997168166
##########
helm/superset/values.yaml:
##########
@@ -267,20 +267,18 @@ supersetNode:
# -- If true, forces deployment to reload on each upgrade
forceReload: false
# -- Init containers
- # @default -- a container waiting for postgres to be up
+ # @default -- a container waiting for postgres
initContainers:
- name: wait-for-postgres
- image: "{{ .Values.initImage.repository }}:{{ .Values.initImage.tag }}"
- imagePullPolicy: "{{ .Values.initImage.pullPolicy }}"
+ image: jwilder/dockerize
Review Comment:
Shouldn't this be a reference to the config keys?
##########
helm/superset/values.yaml:
##########
@@ -267,20 +267,18 @@ supersetNode:
# -- If true, forces deployment to reload on each upgrade
forceReload: false
# -- Init containers
- # @default -- a container waiting for postgres to be up
+ # @default -- a container waiting for postgres
initContainers:
- name: wait-for-postgres
- image: "{{ .Values.initImage.repository }}:{{ .Values.initImage.tag }}"
- imagePullPolicy: "{{ .Values.initImage.pullPolicy }}"
+ image: jwilder/dockerize
+ imagePullPolicy: IfNotPresent
Review Comment:
Same comment as above
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Yann-J commented on pull request #21806: feat(helm): Support for flower and websocket containers
Posted by GitBox <gi...@apache.org>.
Yann-J commented on PR #21806:
URL: https://github.com/apache/superset/pull/21806#issuecomment-1293301321
OK, so I've unblocked the repository dependency issue.
The chart linter now runs successfully, but was raising a tricky error, complaining about whitespaces inside braces in `values.yaml`... the problem is that this linting rule is conflicting with the prettier rules (which is also used to lint YAML) - in fact these spaces were actually introduced by prettier in my VS Code automatically (I have "format on save" enabled).
I've made this linting rule a bit more lax by allowing up to one space, but you may have other opinions on how to fix it.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org