#1154 Update examples notebooks in line with Helm v3 #1188
Conversation
Signed-off-by: glindsell <[email protected]>
examples/seldon_core_setup.ipynb
Outdated
| "metadata": {}, | ||
| "outputs": [], | ||
| "source": [ | ||
| "!helm install seldon-core seldon-core-operator --repo https://storage.googleapis.com/seldon-charts --set usageMetrics.enabled=true --namespace seldon-system --set istio.enabled=true" |
There was a problem hiding this comment.
It seems that the Seldon Core installation command is setting --set istio.enabled=true. Should we remove it to default to Ambassador?
There was a problem hiding this comment.
Have removed helm install command from setup notebook to reduce confusion so that docs are "single source of truth" for helm install command
There was a problem hiding this comment.
Just to clarify, I think that if we have helm install seldon-core ... without the --set istio.enabled=true flag is probably alright as well. We use Ambassador for most examples, so it's probably good to default to it (instead of istio).
This is also a good solution though!
There was a problem hiding this comment.
Sure! I had only put the helm install... command in there to show how to use the --set istio... flag... but it seems that is confusing as it looks like I've put it in to show how to use the install command generally - so for that reason I think it's better to just take it out altogether and reference the install docs which shows how to use it in both instances.
…stall docs Signed-off-by: glindsell <[email protected]>
…stall docs Signed-off-by: glindsell <[email protected]>
…ldon-core into 1154-examples-helm3
|
Fri Dec 6 11:54:45 UTC 2019 impatient try |
|
Fri Dec 6 11:54:51 UTC 2019 impatient try |
|
@ryandawsonuk please take a look at changes discussed today re: bringing helm commands into setup notebook. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ryandawsonuk The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
#1154
Continues from #1187
Signed-off-by: glindsell [email protected]