-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Fixing incorrect deprecation notice #16624
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.
Which team should own what to deprecate? I thought it's service owner team. In addition, we have a property called |
@mbfrahry @sean-yao and I are owning Azure ML RP that Terraform AzureRM is using. We are deprecating allow_public_access_when_behind_virtual_network in favor of public_network_access. Across AzureRM Terraform has forced a boolean type on the public_network_access property which is expecting an Enum {{Enabled, Disabled, PartiallyEnabled}, naming them public_network_access_enabled. The last option is not yet implemented for all services like Azure ML. If public_network_access is not working today, this might be related. FYI @xuzhang3. |
@deeikele can you provide some background on |
PartiallyEnabled == EnabledForSelectedNetworks
On Mon, May 2, 2022 at 21:01 Tom Harvey ***@***.***> wrote:
@deeikele <https://github.com/deeikele> can you provide some background
on PartiallyEnabled?
—
Reply to this email directly, view it on GitHub
<#16624 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMF6GYW5ORFUMBU2N5ZJBH3VICQKRANCNFSM5U5JTE6A>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
|
FYI I seem to have copied the wrong issue link in the code comment, it should be this one Azure/azure-rest-api-specs#18601 |
|
One of the problems with the current Terraform API version, is that setting publicNetworkAccess forces replacement. This is an update property on the backend machine learning API, hence should never force replacement. This should be addressed through a separate item, but at least I recommend getting this doc change out to set right expectations on deprecation. |
The deprecation of this property was resolved in #18279 so closing this PR for now. |
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. |
Fixing a deprecation notice that shows one property to be deprecated in favor of another one, while its actually the other way around.