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/02/11 02:25:12 UTC

[GitHub] [superset] ad-m opened a new pull request #18668: feat: pin version of superset in helm chart

ad-m opened a new pull request #18668:
URL: https://github.com/apache/superset/pull/18668


   ### SUMMARY
   
   I propose to pin version of Superset in Chart. 
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   There are several reasons for this:
   - during the restart of the pod, no automatic update is performed, which may potentially damage the user's data (backup must be performed before the update, etc.) or affect the availability of the application,
   - new versions of the Superset may require an adaptation of the Superset,
   - old versions of Helm Chart will be known to correctly run old versions of Superset,
   
   The downside is that we need to update the Chart and release it in order for users to use the new versions. Currently, however, releases are automated, and verification of the new Superset release that it is compatible with the Chart is even advisable.
   
   In the future, we can think of an automatic version update when the Superset version is changed. For this, the e2e Superset tests could be useful, perhaps taking into account the update process as well.
   
   ### TESTING INSTRUCTIONS
   
   I deployed an updated Chart and verified if pods started and inspected created manifests.
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue: #17540
   - [ ] 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] craig-rueda commented on a change in pull request #18668: feat: pin version of superset in helm chart

Posted by GitBox <gi...@apache.org>.
craig-rueda commented on a change in pull request #18668:
URL: https://github.com/apache/superset/pull/18668#discussion_r804907902



##########
File path: helm/superset/values.yaml
##########
@@ -160,7 +160,7 @@ extraConfigMountPath: "/app/configs"
 
 image:
   repository: apache/superset
-  tag: latest
+  # tag: "v{{ $.Chart.AppVersion }}"

Review comment:
       Could be, but it's worth a try :). I've found that in general, the more fields that support templating, the better.




-- 
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] CarpathianUA commented on pull request #18668: feat: pin version of superset in helm chart

Posted by GitBox <gi...@apache.org>.
CarpathianUA commented on pull request #18668:
URL: https://github.com/apache/superset/pull/18668#issuecomment-1036044394


   @ad-m 
   LGTM, but I suggest waiting for the next release and pinning a new version during releasing it.
   Example use case:
   1. Yesterday I've updated Superset by executing `helm upgrade --install`. Since the image version in `values.yaml` is always `latest` ( I suppose that the `latest` tag is corresponds to the last commit in `master` branch ), I've got Superset image version updated to the latest possible version bound to commit.
   2. After that I've found a `1.4.1` image tag on Docker Hub, and used that tag to update Superset again. After update to `1.4.1` I've got a non-working system - my charts and dashboards got broken ... So if users, who upgraded Superset recently, will update the chart with `1.4.1` most likely they will have issues with Superset after the upgrade. That's why I would prefer to start pinning version of an image with a new release created.
   
   Thanks in advance!


-- 
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] CarpathianUA commented on a change in pull request #18668: feat: pin version of superset in helm chart

Posted by GitBox <gi...@apache.org>.
CarpathianUA commented on a change in pull request #18668:
URL: https://github.com/apache/superset/pull/18668#discussion_r804899195



##########
File path: helm/superset/values.yaml
##########
@@ -160,7 +160,7 @@ extraConfigMountPath: "/app/configs"
 
 image:
   repository: apache/superset
-  tag: latest
+  # tag: "v{{ $.Chart.AppVersion }}"

Review comment:
       BTW, I'm not sure that the referencing  `"v{{ $.Chart.AppVersion }}"` will work at all, since Helm values don't support any templating besides `include` and `template` via plane YAML AFAIR




-- 
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] ad-m commented on pull request #18668: feat: pin version of superset in helm chart

Posted by GitBox <gi...@apache.org>.
ad-m commented on pull request #18668:
URL: https://github.com/apache/superset/pull/18668#issuecomment-1036050886


   @CarpathianUA, we had the latest release of Chart 16 hours ago ( https://github.com/apache/superset/releases/tag/superset-helm-chart-0.5.8 ) and we released the latest Superset version before that ( https://github.com/apache/superset/releases/tag/1.4.1 ). Do I understand correctly that you expect no new Chart to be released until Superset >1.4.1. eg. 1.4.2 is released?


-- 
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] wiktor2200 commented on pull request #18668: feat: pin version of superset in helm chart

Posted by GitBox <gi...@apache.org>.
wiktor2200 commented on pull request #18668:
URL: https://github.com/apache/superset/pull/18668#issuecomment-1036077020


   It sounds great. It also caused some problems for me in the past (as far as I remember it was caused by changing container's entrypoint), but then I've just changed `image: lastest` to selected version, but still having the current stable out-of-the-box would be the best and fail-proof.


-- 
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] wiktor2200 edited a comment on pull request #18668: feat: pin version of superset in helm chart

Posted by GitBox <gi...@apache.org>.
wiktor2200 edited a comment on pull request #18668:
URL: https://github.com/apache/superset/pull/18668#issuecomment-1036077020


   It sounds great. It also caused some problems for me in the past (as far as I remember it was caused by changing container's entrypoint), but then I've just changed `image: lastest` to selected version, but still having the current stable out-of-the-box would be the best and fail-proof.
   I've mentioned it here: https://github.com/apache/superset/issues/17712#issuecomment-1018373201


-- 
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] wiktor2200 commented on a change in pull request #18668: feat: pin version of superset in helm chart

Posted by GitBox <gi...@apache.org>.
wiktor2200 commented on a change in pull request #18668:
URL: https://github.com/apache/superset/pull/18668#discussion_r808325456



##########
File path: helm/superset/Chart.yaml
##########
@@ -15,14 +15,14 @@
 # limitations under the License.
 #
 apiVersion: v2
-appVersion: "1.0"
+appVersion: "1.4.1"
 description: Apache Superset is a modern, enterprise-ready business intelligence web application
 name: superset
 maintainers:
   - name: craig-rueda
     email: craig@craigrueda.com
     url: https://github.com/craig-rueda
-version: 0.5.8
+version: 0.5.9

Review comment:
       ```suggestion
   version: 0.5.10
   ```




-- 
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 change in pull request #18668: feat: pin version of superset in helm chart

Posted by GitBox <gi...@apache.org>.
craig-rueda commented on a change in pull request #18668:
URL: https://github.com/apache/superset/pull/18668#discussion_r804896998



##########
File path: helm/superset/values.yaml
##########
@@ -160,7 +160,7 @@ extraConfigMountPath: "/app/configs"
 
 image:
   repository: apache/superset
-  tag: latest
+  # tag: "v{{ $.Chart.AppVersion }}"

Review comment:
       Can we just use this templated value? I think that adding the ability to template these fields would be useful.
   i.e.:
   ```
   tag: "v{{ $.Chart.AppVersion }}"
   ```
   and then in deployment template, etc:
   ```
   image: "{{ .Values.image.repository }}:{{ tpl .Values.image.tag . }}"
   ```




-- 
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] CarpathianUA commented on pull request #18668: feat: pin version of superset in helm chart

Posted by GitBox <gi...@apache.org>.
CarpathianUA commented on pull request #18668:
URL: https://github.com/apache/superset/pull/18668#issuecomment-1036487453


   @ad-m 
   I mean that until 1.4.2 I would prefer to keep the default image tag in values as it is (latest tag). When a new release will be published (e.g. 1.4.2) - update the default image tag to 1.4.2 by default. 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