Skip to content
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

add storage account config to function apps #16684

Closed

Conversation

Benedikt-Fuchs-Crayon
Copy link
Contributor

adds support for storage_account blocks in azurerm__function_app as described in #6790
also fixing a small copy paste mistake, where a error message falsely claims the wrong os

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pr @Benedikt-Fuchs-Crayon - i think we'lll need to update the docs with the new properties?

@jackofallops jackofallops self-assigned this May 18, 2022
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Benedikt-Fuchs-Crayon - looks like we have some test failures to fix thou:

image

…ame in withStorageAccount and withStorageAccountUpdate methods.
@Benedikt-Fuchs-Crayon
Copy link
Contributor Author

@katbyte thank you for testing it, I sadly didn't manage to setup the acceptance tests myself.
I updated the template which should fix that error for all tests listed, except for TestAccWindowsFunctionApp_addAppSettings. For that one, I don't understand why it is failing, as I didn't touch that test or the respective functionality.

Copy link
Member

@jackofallops jackofallops left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Benedikt-Fuchs-Crayon
Thanks for the changes, there's just a couple test config changes needed and I think this is good to go.

Thanks!

@Benedikt-Fuchs-Crayon
Copy link
Contributor Author

Benedikt-Fuchs-Crayon commented Jul 4, 2022

thank you for the feedback,
I changed the paths to \\mounts\\files and \\mounts\blob respectively

Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Benedikt-Fuchs-Crayon - still seeing some test failures related to this pr:

image

@jackofallops
Copy link
Member

Hi @Benedikt-Fuchs-Crayon - To resolve the conflicts and extend the feature to slots I've pulled your changes onto a new branch/PR (I was unable to safely push to your fork as the PR originated from your main branch, sorry 🙈 ). I'm going to close this PR in favour of the new one, where I've kept as much as I can of your code while resolving the conflicts and extending the functionality to slots.

Thanks again for the contribution, and apologies for the delays in getting back to this!

@github-actions
Copy link

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants