-
Notifications
You must be signed in to change notification settings - Fork 156
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 support for VNet integration #888
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.
Looks great, thank you for the contribution! One small adjustment would be to follow the conventions used in other resources that attach to a subnet.
Also, IIRC there are pricing tier requirements for attaching to a vnet, so it would be good to enforce those.
Thanks for the review. I believe I have now addressed all the comments you have made. |
@TheRSP thanks for updating the operation to |
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.
Is the Standard SKU another name for the consumption plan?
- add further LinkTo*Vnet overloads - update tests
@TheRSP can you please check into the build errors? |
Sorry about that, forgot to update the tests after I enhanced the validation |
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.
Great, thanks for the updates!
Add support for VNet integration bf094e5
This is released in Farmer 1.6.31. |
This PR closes #656 #797
The changes in this PR are as follows:
I have read the contributing guidelines and have completed the following:
If I haven't completed any of the tasks above, I include the reasons why here:
Below is a minimal example configuration that includes the new features, which can be used to deploy to Azure: