Skip to content
This repository has been archived by the owner on Nov 14, 2020. It is now read-only.

postgresql_schema: Add drop_cascade option. #108

Merged
merged 1 commit into from
Dec 23, 2019
Merged

postgresql_schema: Add drop_cascade option. #108

merged 1 commit into from
Dec 23, 2019

Conversation

cyrilgdn
Copy link
Contributor

@cyrilgdn cyrilgdn commented Dec 20, 2019 •
edited
Loading

This allows to delete cascade all dependent objects while deleting a schema.

fix #101

@cyrilgdn cyrilgdn force-pushed the schema_drop_cascade branch 2 times, most recently from 774915e to 4c6ac2f Compare December 20, 2019 16:16
@cyrilgdn cyrilgdn mentioned this pull request Dec 20, 2019
Closed
navi86
navi86 reviewed Dec 21, 2019
View reviewed changes
postgresql/resource_postgresql_schema_test.go Outdated
@@ -196,6 +196,32 @@ func TestAccPostgresqlSchema_Database(t *testing.T) {
})
}

func TestAccPostgresqlSchema_DropCascade(t *testing.T) {
var testAccPostgresqlSchemaConfig = `
resource "postgresql_schema" "test_restrict" {
Copy link

@navi86 navi86 Dec 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resource "test_restrict" but drop_cascade is true
I think it might be better to name resource as "test_cascade"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I changed the test behavior in the middle but not the resource name !

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

@cyrilgdn
postgresql_schema: Add drop_cascade option.
26b84d8 
This allows to delete cascade all dependent objects while deleting a schema.
@cyrilgdn cyrilgdn force-pushed the schema_drop_cascade branch from 4c6ac2f to 26b84d8 Compare December 23, 2019 08:34
@navi86
Copy link

navi86 commented Dec 23, 2019

@cyrilgdn
I've tried to add test for validation that it's not possible to drop schema when it has dependent resource, but I couldn't find solution, because "expectError" works for test steps and there is no way to add it for destroy step. Am I right ?

@cyrilgdn
Copy link
Contributor Author

@cyrilgdn
I've tried to add test for validation that it's not possible to drop schema when it has dependent resource, but I couldn't find solution, because "expectError" works for test steps and there is no way to add it for destroy step. Am I right ?

AFAIK yes, I also tried to create this test (that's why the naming was not coherent) and tried this expectError too but didn't managed to do it.
I'll ask on the Terraform community Slack if there's a way. But for now I just rely on the fact that this test fails if I remove drop_cascade

@cyrilgdn
Copy link
Contributor Author

@navi86 Are you agree to merge it like that? I want to create a new release (with this feature inside) and Hashicorp devs will be OOO for the end of the year so it'd be great if I can ask them today.

@navi86
Copy link

navi86 commented Dec 23, 2019

@navi86 Are you agree to merge it like that? I want to create a new release (with this feature inside) and Hashicorp devs will be OOO for the end of the year so it'd be great if I can ask them today.

sure, LGTM

navi86
navi86 approved these changes Dec 23, 2019
View reviewed changes
Copy link

@navi86 navi86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cyrilgdn cyrilgdn merged commit f0c8d55 into master Dec 23, 2019
@cyrilgdn cyrilgdn deleted the schema_drop_cascade branch December 23, 2019 14:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@navi86 navi86 navi86 approved these changes

Assignees
No one assigned
Labels
documentation size/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

postgresql drop schema
2 participants
@cyrilgdn @navi86