-
Notifications
You must be signed in to change notification settings - Fork 381
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
existing PD integration should not be deleted on create #564 #565
Conversation
Looks like the test changes will require me to re-record the cassettes since the code change will add an additional GET request to the integration endpoint. I"ll do so later with a non-production organization, same for any new test cases |
78149f4
to
8453025
Compare
@@ -150,7 +161,6 @@ func resourceDatadogIntegrationPagerdutyRead(d *schema.ResourceData, meta interf | |||
d.Set("services", services) | |||
d.Set("subdomain", pd.GetSubdomain()) | |||
d.Set("schedules", pd.Schedules) | |||
|
|||
return nil |
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.
return nil | |
return nil |
@@ -112,6 +119,10 @@ func resourceDatadogIntegrationPagerdutyCreate(d *schema.ResourceData, meta inte | |||
return fmt.Errorf("failed to parse resource configuration: %s", err.Error()) | |||
} | |||
|
|||
if exists, _ := resourceDatadogIntegrationPagerdutyExists(d, meta); exists && d.Get("preserve_existing_integration").(bool) { | |||
return translateClientError(errors.New("PagerDuty integration already exists"), "Please import into your existing state or remove from your HCL.") |
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.
@skang0601 can you please add new test to cover this branch? Thank you!
We discussed another approach, thanks for your patch. |
Addressed #564
Added a new optional field in the datadog_pagerduty_integration resource the maintain backwards compatibility with the old behavior but allows users to also prevent the resource from deleting the existing PagerDuty integration silently.
This seems like something that the API should handle by returning an appropriate error status code, but the provider should also follow conventions/expectations for Terraform users regarding resource creation/deletion.