-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
azurerm_mssql_database
/azurerm_mssql_managed_database
- remove Computed for weekly_retention
, monthly_retention
and yearly_retention
#26914
Conversation
…y_retention and yearly_retention
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 @neil-yechenwei. Are there no sensible defaults that we can set here or can the default change depending on the config?
@stephybun , thanks for the comment. I added default value. Below is the test result I just now triggered. Below failed test cases are also failed with same error on Teamcity Daily Run. So they're not related with 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.
we are now in 4.0 so i think this needs to be updated
@katbyte , thanks for the reminder. I updated PR for v5.0. |
azurerm_mssql_database
- remove Computed for weekly_retention
, monthly_retention
and yearly_retention
azurerm_mssql_database
/azurerm_mssql_managed_database
- remove Computed for weekly_retention
, monthly_retention
and yearly_retention
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.
LGTM 🏛️
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. |
Community Note
Description
As
weekly_retention
,monthly_retention
andyearly_retention
are added the Computed attribute, so TF wouldn't cause difference when they are removed from tf config. Per the latest Hashicorps's suggestion, we should remove Computed.PR Checklist
For example: “
resource_name_here
- description of change e.g. adding propertynew_property_name_here
”Changes to existing Resource / Data Source
Testing
Below failed test cases are also failed with same error on Teamcity Daily Run. So they're not related with this PR.
MSSQL Database (Below failed test cases are also failed with same error on Teamcity. So they are not related with this PR.):
MSSQL Managed Database (Below failed test case is also failed with same error on Teamcity. So it's not related with this PR.):
Change Log
Below please provide what should go into the changelog (if anything) conforming to the Changelog Format documented here.
azurerm_mssql_database
/azurerm_mssql_managed_database
- remove Computed forweekly_retention
,monthly_retention
andyearly_retention
This is a (please select all that apply):
Related Issue(s)
Fixes #26906
Note
If this PR changes meaningfully during the course of review please update the title and description as required.