-
Notifications
You must be signed in to change notification settings - Fork 200
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
Add support for view resource #442
base: main
Are you sure you want to change the base?
Conversation
Thanks for putting this together, it looks great and I hope this gets merged. One potential issue that I see is that sometimes views cannot be replaced with a mere create or replace statement. See below for example. Is this resource resilient to this issue? The quick and dirty way to make it resilient to it is to issue a DROP VIEW IF EXISTS command before the CREATE VIEW command. This should have very little downtime as the query can be dropped and created very quickly. The more complete method would be to parse the change and issue ALTER VIEW commands, granted this would take a lot more work.
|
Thank you @AndrewJackson2020 for your review. The issue is fixed. I'll be great if you can take a look again! |
Looks good, I would just throw in a unit test that covers the case I outlined above. Also I would get rid of the "create or replace" since ur function is already deleting it so the "or replace" part is not necessary anymore. Was also looking at your implementation of read. The comments make it sound like if there is any drift that was not caused by terraform then manual intervention is required. Is this the case? If so why? |
Thank you @AndrewJackson2020 for having a look. For the drift, it might happen when multiple people have modified access to the view and they just directly modify the view in Postgres without Terraform awareness. If it happens I think we should let them(the people who make the change and the people who manage Postgres via Terraform) decide whether to keep the change in Postgres or replace it with what in Terraform. |
So its just a warning? Nothing prevents the user from overriding the changes with terraform. I think that makes sense. I think that is all I have for your PR hopefully the maintainer will be able to review and merge. |
Really appreciate your review! Thank you @AndrewJackson2020 |
@cyrilgdn & @nguyen1tech thanks both for working on this module. It's super useful and I am really loving it so far. Thanks all for your time spent on this ❤️ |
f2c2e47
to
dea1401
Compare
This PR adds support for views in the provider.
Features:
Notes: This PR is only for regular views, materialized views deserve a separate resource type. eg: postgresql_materialized_view
Please let me know if you see any issues!