-
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
Introduce OracleDatabase ExaData Infrastructure Resource & Data #27177
base: main
Are you sure you want to change the base?
Introduce OracleDatabase ExaData Infrastructure Resource & Data #27177
Conversation
internal/services/oracledatabase/exadata_infrastructure_resource.go
Outdated
Show resolved
Hide resolved
website/docs/r/oracledatabase_exadata_infrastructure.html.markdown
Outdated
Show resolved
Hide resolved
website/docs/r/oracledatabase_exadata_infrastructure.html.markdown
Outdated
Show resolved
Hide resolved
website/docs/r/oracledatabase_exadata_infrastructure.html.markdown
Outdated
Show resolved
Hide resolved
// WebsiteCategories returns a list of categories which can be used for the sidebar | ||
func (r Registration) WebsiteCategories() []string { | ||
return []string{ | ||
"App Service", |
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 is wrong?
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.
Fixed, thanks!
|
||
// Name is the name of this Service | ||
func (r Registration) Name() string { | ||
return "App Service" |
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.
and here?
var output ExadataInfraResourceModel | ||
|
||
output.CustomerContacts = FlattenCustomerContacts(result.Model.Properties.CustomerContacts) | ||
output.Name = pointer.ToString(result.Model.Name) | ||
output.Location = result.Model.Location | ||
output.Zones = result.Model.Zones | ||
output.ResourceGroupName = id.ResourceGroupName | ||
output.Tags = utils.FlattenPtrMapStringString(result.Model.Tags) | ||
prop := result.Model.Properties | ||
output.ComputeCount = pointer.From(prop.ComputeCount) | ||
output.DisplayName = prop.DisplayName | ||
output.StorageCount = pointer.From(prop.StorageCount) | ||
output.Shape = prop.Shape | ||
output.MaintenanceWindow = FlattenMaintenanceWindow(prop.MaintenanceWindow) |
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.
minor issue but to match other resources we should inline this/do an assingment
output := ExadataInfraResourceModel {
CustomerContacts: FlattenCustomerContacts(result.Model.Properties.CustomerContacts)
Name: pointer.ToString(result.Model.Name)
...
and such vs going outout.x
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.
Understood, I've updated the code.
Optional: true, | ||
Computed: true, | ||
Elem: &pluginsdk.Schema{ | ||
Type: pluginsdk.TypeInt, |
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 should be validated between 1-5?
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.
Internally we want 1-4, I've updated to validate that, thank you.
"preference": { | ||
Type: pluginsdk.TypeString, | ||
Optional: true, | ||
Computed: true, |
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 should be validated?
Optional: true, | ||
Computed: true, | ||
Elem: &pluginsdk.Schema{ | ||
Type: pluginsdk.TypeString, |
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.
and here?
please validate all properties
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!
output.Name = id.CloudExadataInfrastructureName | ||
output.ResourceGroupName = id.ResourceGroupName | ||
output.Type = pointer.From(model.Type) | ||
output.Tags = utils.FlattenPtrMapStringString(model.Tags) | ||
output.Location = model.Location | ||
output.Zones = model.Zones |
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.
why are these seperate and not included above?
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.
I've moved them up with the test.
…down Co-authored-by: kt <[email protected]>
…down Co-authored-by: kt <[email protected]>
…down Co-authored-by: kt <[email protected]>
… into introduce-exa-data-and-resource
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.
I think this now looks good provided CI passes, please show confirmation of passing tests and we can get this merged
…e-exa-data-and-resource
…e-exa-data-and-resource
Community Note
Description
PRE-REVIEW - PLEASE DO NOT MERGE
Introduce OracleDatabase ExaData Infrastructure Resource & Data
Most relevant changes in:
internal/services/oracledatabase/
PR Checklist
For example: “
resource_name_here
- description of change e.g. adding propertynew_property_name_here
”Testing
ExadataInfraDataSource
test:ExadataInfraResource
tests:Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
This is a (please select all that apply):
Note
If this PR changes meaningfully during the course of review please update the title and description as required.