-
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_mysql_flexible_server_configuration
- Remove ForceNew
for value
#22557
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.
Hi @neil-yechenwei, thanks for this PR.
Can you create separate functions for Create and Update? In the future we'll need these to be correctly separated, so we shouldn't introduce further re-use here (even if the code is the same today).
Thanks
@@ -20,8 +20,9 @@ import ( | |||
|
|||
func resourceMySQLFlexibleServerConfiguration() *pluginsdk.Resource { | |||
return &pluginsdk.Resource{ | |||
Create: resourceMySQLFlexibleServerConfigurationCreate, | |||
Create: resourceMySQLFlexibleServerConfigurationUpdate, |
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.
Can we have separate Create and Update methods here and not re-use, we will need this going forward in any case.
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.
@jackofallops , thanks for the comments. I've updated PR. Please take another look. 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.
Thanks for the updates @neil-yechenwei, just one comment in the tests and I think this will be good to go.
Thanks
Config: r.template(data, "slow_query_log", "OFF"), | ||
Check: acceptance.ComposeTestCheckFunc( | ||
data.CheckWithClient(r.checkValue("OFF")), | ||
), | ||
}, | ||
data.ImportStep(), | ||
{ | ||
Config: r.template(data, "slow_query_log", "ON"), |
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.
Can we follow the pattern here in the other tests rather than specifying the template directly? e.g. Config: r.characterSetServer(data),
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.
@jackofallops , thanks for the comment. I've updated PR. Please take another look. 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.
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. |
fixes #22526