Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
In other SQL related resources we set this field during Import, since this is an Import specific issue - can we do that here too?
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.
@tombuildsstuff , I remember previously setting "d.Set("create_mode", d.Get("create_mode").(string))" in read func can set the property with the value in the tf config to handle the issue of not returning the value by service API but now seems it still sets the value as null in state file while importing the resource. I tried below fixes but all they don't work. Is there any suggestion about how to fix the issue of API doesn't return the value while importing the resource?
Fix # 1. I tried to add "d.Set("create_mode", d.Get("create_mode").(string))" in read func but TF still always sets it as "null" in state file when importing the resource.
Fix # 2. I tried to add d.set() with d.Get() to importer but TF still always sets it as "null" in state file when importing the 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.
For this kind of issue, seems only ignore_changes can workaround this issue for now. So I will close this PR.
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.
@neil-yechenwei as I mentioned in my original comment, if you refer to the other places where we set
create_mode
at import time we set a value for this field:terraform-provider-azurerm/internal/services/mssql/mssql_database_resource.go
Lines 129 to 132 in 2e7a613
As such this is possible to provide a default value for this field, and then override it from the config as needed.
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.
@tombuildsstuff , Here "the config" you mentioned points to the tf config file or the configurations returned by service API?
If here "the config" you mentioned points to the tf config file, I tried to use d.Get() in the custom importer but it can't get the value from the tf config when importing the resource.
If here "the config" you mentioned points to the configurations returned by service API, I assume that we can't differentiate create modes "PointInTimeRestore" and "Default" and "GeoRestore" for the PITR resource and the Default resource and the GeoRestore resource since API always returns replicationRole "None" for them. (Note: replicationRole is the unique property to distinguish create mode.)
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.
@neil-yechenwei
Rather than assuming, please validate the API behaviour for each type of CreateMode.
Whilst you're correct insofar as
PointInTimeRestore
andGeoRestore
are going to be similar, the API responses mentioned above should allow us to do something similar to this:As such would you be able to log the API responses in each case here, so that we can determine how to proceed?
Thanks!
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.
@tombuildsstuff , Below is the test result. Except "create_mode = Replica", seems we can't distinguish other create modes from the service configuration.
Below is the response when creating mysql fs with create_mode = "Default" or not set create_mode.
Below is the response when creating mysql fs with create_mode = "PointInTimeRestore".
Below is the response when creating mysql fs with create_mode = "Replica".
Below is the response when creating mysql fs with create_mode = "GeoRestore".