fix: Add MODEL_NAME to kind deploy#178
Conversation
Signed-off-by: Kfir Toledo <kfir.toledo@ibm.com>
| targetModels: | ||
| - name: food-review | ||
| - name: ${MODEL_NAME} | ||
| weight: 100 |
There was a problem hiding this comment.
nit: if modelName equals targetModels (meaning target models array has only one entry with name identical to spec.modelName), you can just remove the targetModels part (it's optional and is typically used for traffic split).
There was a problem hiding this comment.
I see, but I still prefer to keep it, because I will use it as a reference
scripts/kind-dev-env.sh
Outdated
| fi | ||
| kustomize build --enable-helm ${KUSTOMIZE_DIR} \ | ||
| | envsubst \${POOL_NAME} | envsubst \${EPP_TAG} | envsubst \${VLLM_SIMULATOR_TAG} \ | ||
| | envsubst \${POOL_NAME} | envsubst '${MODEL_NAME}' | envsubst \${EPP_TAG} | envsubst \${VLLM_SIMULATOR_TAG} \ |
There was a problem hiding this comment.
'${MODEL_NAME}' doesn't have the precending \ like others have, plus it has '' around it while other vars don't. is it intentional?
additionally, have you considered using just envsubst without specifying the variables one by one?
There was a problem hiding this comment.
Fix, I just added variables here:) I think the main reason is that parts of the YAML contain variables that are set at runtime, and you don't want to be overridden from outside(if it is not defined, it will be empty with envsubst.
Signed-off-by: Kfir Toledo <kfir.toledo@ibm.com>
|
@shmuelk and @nirrozenbaum PTAL. |
4 participants
Add MODEL_NAME to the deploy scripts, and allow it to be changed.