Correctly show function app resources in provision log#1503
Correctly show function app resources in provision log#1503weikanglim merged 1 commit intoAzure:mainfrom
Conversation
Function Apps and App Service apps use the same Azure resource types, so our display logic needs to query information about the provisioned resource to understand if a provisioned resource is an app service or a function app. Our existing logic to do this was broken when we moved away from using `az` to get information about a resource, since we were not passing an API version on our query. This caused our REST call to fail, and we just ignored the error (under the assumption it was a transient issue and we were okay displaying slightly wrong output for progress in this case). This change addresses the issue by forcing callers (who in theory understand what API version is valid for the resource they want to fetch information about) to provide an API verison and updates the one call site we have to pass a valid API version. Without this change, when deploying a template like `todo-nodejs-mongo-swa-func` you'd see this in the provision log: ``` (✓) Done: Web App: func-api-trxnle2vswr24 ``` Whereas with this change, we now correctly report that this is a function app: ``` (✓) Done: Function App: func-api-trxnle2vswr24 ``` Fixes Azure#1484
|
@weikanglim - This was a quick fix for the issue, I have a branch locally where I also added a Trying to think about the best way to write a test for this - I could imagine trying to write a unit test for the display logic and mock returning a deployment status object with both an app service and a function app and make sure the display logs the two lines correctly (after mocking the calls that will happen for |
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSIContainerDocumentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference (preview)
|
weikanglim
left a comment
There was a problem hiding this comment.
LGTM. Thanks for fixing this!
|
@ellismg My first question would be if we cared enough about the accuracy of resource display. I could see it being acceptable, both as a product owner and as a product user, that these types of issues end up getting filed and fixed by the product team. If we want to invest effort here, I think we should be tactical about it, which means one of two things:
Otherwise, what broke here is the client-service contract. Specifically, the contract wasn't honored by the client. These are not things that can easily change, and for most cases, you would treat these as axiomatic. If they do change, you do want a test that catches the breakage. And thus, it can only be produced by verifying the actual client-server behavior, and not by mocked responses that would not catch a change in the service. Also, to note, the thing I'd care most about is that we make it impossible or near impossible to form an invalid client request. That seems to be taken care of with the change. |
Function Apps and App Service apps use the same Azure resource types, so our display logic needs to query information about the provisioned resource to understand if a provisioned resource is an app service or a function app.
Our existing logic to do this was broken when we moved away from using
azto get information about a resource, since we were not passing an API version on our query. This caused our REST call to fail, and we just ignored the error (under the assumption it was a transient issue and we were okay displaying slightly wrong output for progress in this case).This change addresses the issue by forcing callers (who in theory understand what API version is valid for the resource they want to fetch information about) to provide an API verison and updates the one call site we have to pass a valid API version.
Without this change, when deploying a template like
todo-nodejs-mongo-swa-funcyou'd see this in the provision log:Whereas with this change, we now correctly report that this is a function app:
Fixes #1484