-
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
[New Resource] - azurerm_machine_learning_datastore
#19867
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 for this PR @xuzhang3, this will be a great addition to machine learning services! There are however a few larger changes that need to happen before we can consider introducing this resource.
- Since this resource can support multiple types, we should actually split this out into a resource for each type - the user experience will be better and more consistent.
- The resources need to be typed, since they will need to be typed in the future for resource generation from Pandora.
"resource_group_name": commonschema.ResourceGroupName(), | ||
|
||
"workspace_name": { |
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.
We can reduce these two properties down to one workspace_id
ValidateFunc: validate.WorkspaceName, | ||
}, | ||
|
||
"type": { |
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.
We shouldn't expose type, this resource should probably be split out into a separate resource for each type. In addition Data Lake Gen 1 is EOL, so perhaps we should only consider creating a resource for this if explicitly requested?
@stephybun thanks for reviewing this PR, I will close this PR and submit another PR for different types of this resources |
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. |
Fixes: #14701