-
Notifications
You must be signed in to change notification settings - Fork 14
624 new resource netapp ontap protocols fpolicy event #626
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
624 new resource netapp ontap protocols fpolicy event #626
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.
provider.tf should be a link pointing to https://github.com/NetApp/terraform-provider-netapp-ontap/blob/integration/main/examples/provider/provider.tf
If there are some new clusters, please update the canvas on the dev-terraform-ontap channel
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.
done
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.
it is still a file not a link
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.
This file should be the link pointing to https://github.com/NetApp/terraform-provider-netapp-ontap/blob/integration/main/examples/provider/variables.tf
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.
done
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.
It is still a file not a link.
| stringvalidator.OneOf("cifs", "nfsv3", "nfsv4"), | ||
| }, | ||
| }, | ||
| "file_operations": schema.SetAttribute{ |
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.
https://docs.netapp.com/us-en/ontap-restapi-96/ontap/swagger-ui/index.html#/NAS/fpolicy_event_create
The file_operations is a list. Can you double check?
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.
yes, we add Set for a list in terraform so that it supports schema appropriately
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.
if orders does not matter, use set.
| MarkdownDescription: "File operations to be monitored", | ||
| Optional: true, | ||
| }, | ||
| "filters": schema.SetAttribute{ |
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.
Same as filters. It should be a list. https://docs.netapp.com/us-en/ontap-restapi-96/ontap/swagger-ui/index.html#/NAS/fpolicy_event_create
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.
yes, we add Set for a list in terraform so that it supports schema appropriately
| MarkdownDescription: "Filters to be applied for the specified file operations", | ||
| Optional: true, | ||
| }, | ||
| "volume_monitoring": schema.BoolAttribute{ |
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.
If it is not provided, there will be a value to be set.
Can this add:
PlanModifiers: []planmodifier.Bool{
boolplanmodifier.UseStateForUnknown(),
},
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.
actually the API adds false as default, updated accordingly
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.
On the doc, "If not specified in POST, the following default property values are assigned:
file_operations.* - false
filters.* - false
volume-monitoring - false"
Which means the paramters do not have to be set with default values, the API will take the call without these default values and the response will give the full set with the default values if the parameters are not set in the POST.
chuyich
left a comment
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.
Please double check the parameters on the doc https://docs.netapp.com/us-en/ontap-restapi-96/ontap/swagger-ui/index.html#/NAS/fpolicy_event_create
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.
Please check the comments and fix accordingly. And can you provide some edge test cases and show the result in the PR? Like run only the required parameters on creating and do the update on that. That will help to speed up the review. Thank you.
|
Please check the ACC run in pipeline, it has updated list of tests |
…otocols_fpolicy_event
…otocols_fpolicy_event
| return f | ||
| } | ||
|
|
||
| // // convertFileOperationsFromStruct converts FpolicyEventFileOperations struct to types.Set |
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.
Remove if not needed. If need, add comments.
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.
done
https://github.com/NetApp/terraform-provider-netapp-ontap into 624-new-resource-netapp-ontap_protocols_fpolicy_event
No description provided.