New version 2.1.1 for the bosh-setup template #2652
Conversation
| "typeHandlerVersion": "1.4", | ||
| "publisher": "Microsoft.Azure.Extensions", | ||
| "type": "CustomScript", | ||
| "typeHandlerVersion": "2.0", | ||
| "settings": { | ||
| "fileUris": "[variables('filesToDownload')]" | ||
| }, |
There was a problem hiding this comment.
@ahmetalpbalkan
CustomScript is upgraded to 2.0 in this commit.
| echo "Successfully." | ||
| break | ||
| else | ||
| let retry=retry+1 |
There was a problem hiding this comment.
maybe add a log here as well echo Retrying?
| echo "Operation: $1, Retry #${retry}" | ||
| eval $1 | ||
| if [ $? -eq 0 ]; then | ||
| echo "Successfully." |
There was a problem hiding this comment.
Successful sounds a bit better?
| azure storage blob upload manifests/single-vm-cf.yml ${container_name} ${template_version}/manifests/single-vm-cf.yml | ||
| azure storage blob upload manifests/multiple-vm-cf.yml ${container_name} ${template_version}/manifests/multiple-vm-cf.yml | ||
| directories="scripts manifests" | ||
| for directory in $directories |
There was a problem hiding this comment.
these two scripts look like they're written by different people :) I guess mostly because do statements are on a separate line, unlike retry.sh.
|
Thanks! |
|
@ahmetalpbalkan |
|
@bmoore-msft |
| @@ -113,10 +113,10 @@ | |||
|
|
|||
| "baseUriAzureCloud": "https://raw.githubusercontent.com/Azure/azure-quickstart-templates/master/bosh-setup/", | |||
There was a problem hiding this comment.
Why are we hardcoding this instead of using the best practice from here: https://github.com/Azure/azure-quickstart-templates/blob/master/1-CONTRIBUTION-GUIDE/best-practices.md#samples-that-contain-extra-artifacts-custom-scripts-nested-templates-etc
There was a problem hiding this comment.
@bmoore-msft
From my understanding, the BP for the artifacts are mainly for the template developers. Because for end users, mostly they will use the artifacts we provide, and won't change them. Specially, for the bosh-setup template, the purpose is POC of Cloud Foundry deployment on Azure. And too many parameters may confuse users. So we try to make it simple.
When our team are developing and testing the template, we just change the hardcoding URL to the private location. I think, this way works well.
There was a problem hiding this comment.
@bmoore-msft
Do you have any updates? Does my understanding make sense? Thanks for your kindly reviewing.
There was a problem hiding this comment.
We want to have templates that work well for developers as well as end-users, showing how templates can be flexible and used across clouds with "standard" tools and scripts. You can default the parameter values as described in the bp guide and portal users won't need to worry about it.
There was a problem hiding this comment.
I understand I can give the default value to the parameter. But from our point of view, we don't want to expose the baseUriAzureCloud as a parameter to users because most end-users don't need to change the url.
If this best practice is not a MUST, could you please merge the PR at this point? And we can investigate whether we should expose it or not. If users ask for it, we can expose it in future.
There was a problem hiding this comment.
It is a must we want a consistent staging experience - we've updated BP and PR templates to help with messaging.
There was a problem hiding this comment.
You mentioned the templates are used across clouds with "standard" tools and scripts. What are the tools? From our understanding, we only need to make sure Azure Portal, CLI and Powershell can deploy the template successfully.
We still think this bp is not applicable to the bosh-setup template because end users never need to change the baseUriAzureCloud as a parameter.
There was a problem hiding this comment.
We agree there are a set of users that don't need to change the parameter value - that's why the BP details an approach that provides a default value for those users. That same approach also works for other consumers of the repo...
|
@singhkays Any concerns or updates? |
Changelog