freeznet commented on a change in pull request #11213: URL: https://github.com/apache/pulsar/pull/11213#discussion_r664204818
########## File path: site2/docs/helm-deploy.md ########## @@ -346,7 +346,7 @@ In this example, we name our Helm release `pulsar`. ```bash helm repo add apache https://pulsar.apache.org/charts helm repo update -helm upgrade --install pulsar apache/pulsar \ +helm install pulsar apache/pulsar \ --timeout 10m \ --set [your configuration options] Review comment: ```suggestion helm install pulsar apache/pulsar \ --timeout 10m \ --set initialize=true \ --set [your configuration options] ``` since the `install` command is for the first time deployment, so I suggest adding `--set initialize=true` by default. ########## File path: site2/docs/helm-deploy.md ########## @@ -346,7 +346,7 @@ In this example, we name our Helm release `pulsar`. ```bash helm repo add apache https://pulsar.apache.org/charts helm repo update -helm upgrade --install pulsar apache/pulsar \ +helm install pulsar apache/pulsar \ --timeout 10m \ --set [your configuration options] Review comment: yes, but I think the initial idea of the note is, `helm upgrade --install` is suitable for both first time deployment and upgrade deployment, so according to the note, the user needs to set `--set initialize=true` to init the metadata if it is the first time deployment. With your change, `helm install` is always for the first time deployment, so I suggest adding `--set initialize=true` since it is not an optional config but a must-have one. ########## File path: site2/docs/helm-deploy.md ########## @@ -346,7 +346,7 @@ In this example, we name our Helm release `pulsar`. ```bash helm repo add apache https://pulsar.apache.org/charts helm repo update -helm upgrade --install pulsar apache/pulsar \ +helm install pulsar apache/pulsar \ --timeout 10m \ --set [your configuration options] Review comment: @Anonymitaet please refer to my previous comment. `helm upgrade --install` can be used at the first time deployment and future upgrade, so we can have a note tells the user if you are on the first time deployment, please remember to add `--set initialize=true` because it is required for the first time deployment; if you already have pulsar charts installed, then `--set initialize=true` is not required. But `helm install` is only for the first time deployment, and it is required to have `--set initialize=true`. So IMO, if changed from `helm upgrade --install` to `helm install`, then you have to include `--set initialize=true` in the code block. ########## File path: site2/docs/helm-deploy.md ########## @@ -346,7 +346,7 @@ In this example, we name our Helm release `pulsar`. ```bash helm repo add apache https://pulsar.apache.org/charts helm repo update -helm upgrade --install pulsar apache/pulsar \ +helm install pulsar apache/pulsar \ --timeout 10m \ --set [your configuration options] Review comment: Also, please check PR #9070 -- 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]
