Modifying template to dynamically generate the WebApp endpoints #2571
Conversation
|
Hi @dilukose, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!
TTYL, AZPRBOT; |
| "resources": [ | ||
| { | ||
| "apiVersion": "[variables('apiVersion')]", | ||
| "name": "[concat('DemoAppServicePlan', copyIndex())]", |
There was a problem hiding this comment.
a better practice here would be to concat the location name - use the format for location that doesn't have spaces..
concat('DemoAppServicePlan-', variables('webAppLocations')[copyIndex()])
There was a problem hiding this comment.
removed this change it since its not needed
There was a problem hiding this comment.
Is there are reason we cannot do this? the sample uses geo location for demonstrating TM - would we expect developers to refer to resources by index or by location in this case (customer templates I've seen are more descriptive when labeling resources)
There was a problem hiding this comment.
want to keep this simple
There was a problem hiding this comment.
Ok, simplify it further by removing the uniqueDnsNameForWebApp (and generate the name) and we should be good to go...
| "farmName": "default" | ||
| }, | ||
| "variables": { | ||
| "tmApiVersion": "2015-11-01", |
There was a problem hiding this comment.
use literal values for apiVersions - otherwise schema validation cannot be done
| }, | ||
| { | ||
| "apiVersion": "[variables('apiVersion')]", | ||
| "name": "[concat(parameters('uniqueDnsNameForWebApp'), copyIndex())]", |
There was a problem hiding this comment.
same comment as on serverFarm above...
There was a problem hiding this comment.
removed this change it since its not needed
There was a problem hiding this comment.
Is there are reason we cannot do this? the sample uses geo location for demonstrating TM - would we expect developers to refer to resources by index or by location in this case (customer templates I've seen are more descriptive when labeling resources)
There was a problem hiding this comment.
want to keep this simple
| "tmApiVersion": "2015-11-01", | ||
| "apiVersion": "2015-08-01", | ||
| "webAppLocations": [ "West US", "East US" ], | ||
| "farmName": "default" |
| }, | ||
| "uniqueDnsNameForWebApp": { |
There was a problem hiding this comment.
since TM is the endpoint, do we need to have the user supply a name? Or can we just generate one?
There was a problem hiding this comment.
Yes, the user needs to specify a DNS name
There was a problem hiding this comment.
Why, what's the scenario that requires the user input here?
| ] | ||
| } | ||
| "monitorConfig": { | ||
| "protocol": "http", |
There was a problem hiding this comment.
schema is expecting HTTP here (all caps)
| } | ||
| }, | ||
| { | ||
| "apiVersion": "[variables('tmApiVersion')]", |
There was a problem hiding this comment.
make this a child resource...
There was a problem hiding this comment.
changed this to be a literal. not sure what you mean by child resource
| "count": "[length(variables('webAppLocations'))]" | ||
| }, | ||
| "properties": { | ||
| "targetResourceId": "[resourceId('Microsoft.Web/sites/',concat(parameters('uniqueDnsNameForWebApp'), copyIndex()))]", |
There was a problem hiding this comment.
add whitespace after comma
| "value": "GEN-UNIQUE" | ||
| } | ||
| "uniqueDnsName": { | ||
| "value": "GEN-UNIQUE-1" |
Contributing guide
https://github.com/Azure/azure-quickstart-templates/blob/master/1-CONTRIBUTION-GUIDE/README.md
Changelog
Description of the change