Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[fix] Extra checks for resources in statefile not found (#188) #305

Closed
wants to merge 1 commit into from
Closed

[fix] Extra checks for resources in statefile not found (#188) #305

wants to merge 1 commit into from

Conversation

edwinbdr
Copy link
Contributor

  • Adds checks when resources found in the state file are not found in the Snowflake environment. The resources are then removed from the statefile and a warning is produced.

Test Plan

  • acceptance tests
  • …

References

@edwinbdr
[fix] Extra checks for resources in statefile not found (#188)
536b158 
* Adds checks when resources found in the state file are not found in the Snowflake environment. The resources are then removed from the statefile and a warning is produced.
@edwinbdr edwinbdr requested a review from a team as a code owner November 19, 2020 08:17
@edwinbdr edwinbdr requested review from edulop91 and removed request for a team November 19, 2020 08:17
@lgwacker
Copy link
Contributor

Great!!

ryanking
ryanking previously requested changes Nov 19, 2020
View reviewed changes
Copy link
Contributor

@ryanking ryanking left a comment

Choose a reason for hiding this comment

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

Thank you for this, it makes sense. Would it be possible to write some tests for this? I know it might seem a bit laborious, but making changes to this project without writing tests has been biting us pretty hard.

@lgwacker
Copy link
Contributor

@ryanking if you could give me some guidance on how to build those tests, I would be glad to write them.

@lgwacker
Copy link
Contributor

lgwacker commented Nov 26, 2020 •
edited

@edwinbdr I opened a PR to your fork with the tests. Can you merge it and add it to this PR?
https://github.com/centrient-dna/terraform-provider-snowflake/pull/1

@lgwacker lgwacker mentioned this pull request Dec 8, 2020
Merged
ryanking added a commit that referenced this pull request Dec 9, 2020
@lgwacker @edwinbdr @ryanking
[fix] Remove resource from state file if its not found (#339)
54407e3 
This PR is a redo of #305, since it was missing tests and some resources were not working.

Fixes #188 

Co-authored-by: Edwin De Jong <edwin.de.jong@bigdatarepublic.nl>
Co-authored-by: Ryan King <rking@chanzuckerberg.com>
@ryanking ryanking dismissed their stale review March 9, 2021 18:35

handing off to allison

gjv9491 pushed a commit to gjv9491/terraform-provider-snowflake that referenced this pull request Mar 19, 2021
@lgwacker @edwinbdr @ryanking
[fix] Remove resource from state file if its not found (Snowflake-Lab…
c9ef665 
…s#339)

This PR is a redo of Snowflake-Labs#305, since it was missing tests and some resources were not working.

Fixes Snowflake-Labs#188 

Co-authored-by: Edwin De Jong <edwin.de.jong@bigdatarepublic.nl>
Co-authored-by: Ryan King <rking@chanzuckerberg.com>
@alldoami
Copy link
Contributor

alldoami commented Mar 29, 2021 •
edited

Hi @lgwacker can you fix the conflicts and I will take a look at this again?

@lgwacker
Copy link
Contributor

Hi, @alldoami I guess this PR can be closed. I created another with these changes and tests and it was merged already.
#339

@alldoami alldoami closed this Mar 30, 2021
anton-chekanov pushed a commit to anton-chekanov/terraform-provider-snowflake that referenced this pull request Jan 25, 2022
@lgwacker @edwinbdr
@ryanking @anton-chekanov
[fix] Remove resource from state file if its not found (Snowflake-Lab…
97555a0 
…s#339)

This PR is a redo of Snowflake-Labs#305, since it was missing tests and some resources were not working.

Fixes Snowflake-Labs#188 

Co-authored-by: Edwin De Jong <edwin.de.jong@bigdatarepublic.nl>
Co-authored-by: Ryan King <rking@chanzuckerberg.com>
daniepett pushed a commit to daniepett/terraform-provider-snowflake that referenced this pull request Feb 9, 2022
@lgwacker @edwinbdr @ryanking
[fix] Remove resource from state file if its not found (Snowflake-Lab…
55e0c6d 
…s#339)

This PR is a redo of Snowflake-Labs#305, since it was missing tests and some resources were not working.

Fixes Snowflake-Labs#188 

Co-authored-by: Edwin De Jong <edwin.de.jong@bigdatarepublic.nl>
Co-authored-by: Ryan King <rking@chanzuckerberg.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ryanking ryanking ryanking left review comments

@edulop91 edulop91 Awaiting requested review from edulop91

@alldoami alldoami Awaiting requested review from alldoami

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@edwinbdr @lgwacker @alldoami @ryanking