-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
azurerm_windows_function_app
/azurerm_linux_function_app
- Add support for PowerShell 7.2
#16718
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Francisco-Gamino - LGTM but could we update the docs with the new valid value? should be good to merge after that sis done! thanks
Thank you @katbyte for the PR review. Could you please point me to the doc that needs to be updated? I am not very familiar with your repro as this is my first contribution. |
f078f03
to
329ae7a
Compare
Hello @katbyte -- I did a recursive search for any docs that contain
Please let me know if I am missing anything. Lastly, once this PR is merged, how long will it take to release this change? Thank you! |
Thank you @mbfrahry for fixing my PR. |
azurerm_windows_function_app
/azurerm_linux_function_app
- Add support for PowerShell 7.2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a problem @Francisco-Gamino! LGTM!
Thank you @mbfrahry ! One last question: When will this change be released? |
This functionality has been released in v3.7.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
After setting the parameter to 7.0 or 7.2, should that value be reflected in the Azure portal? We have tried both versions and under Configuration>General Settings the Stack is listed as PowerShell Core but the PowerShell Core Version option is blank. The versions are available in the drop-down, just neither is selected. Is this possibly a situation where there is just a bug in the GUI and the setting is actually set on the backend? |
Hello @bdorplatt -- I am not sure if the Azure Portal has a hardcoded or dynamic list of supported language versions. I would suggest opening an issue against the Azure Portal. The support for |
While support was added to the code, it doesn't appear to actually set the value of the PowerShell version. Changing the version back and forth from 7 to 7.2 in code doesn't seem to modify the actual resource. The same thing happens when using Java as the application stack but works correctly with Node and reflects the correct version in the portal. It appears to be the same issue as here https://github.com/hashicorp/terraform-provider-azurerm/issues/8867 |
@bdorplatt -- The code change adds
These is how the app settings for my app look like after deploying: |
We were under the impression that the new application_stack block settings superseded the FUNCTIONS_WORKER_RUNTIME and FUNCTIONS_WORKER_RUNTIME_VERSION app settings and would fill the version into the app settings. It is doing this for FUNCTIONS_WORKER_RUNTIME, filling "powershell" into the app settings without us specifying it as an app setting. The only app settings we are specifying are below: And we get the following result in app settings, where only "powershell" without the version is showing. It seems odd that the application stack settings would have been added but you have to specify the versions in two different places. (in stack_settings as well as app_settings) |
@bdorplatt -- Thank you for the reply. I am new to Terraform, so I am going to get let a Terraform SME answer this. |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions. |
Resolves #16717.