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.

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

Update failover_group.md allowed_integration_types #2518

Closed
wants to merge 1 commit into from

Conversation

gdelia-pm
Copy link

Added STORAGE INTEGRATIONS to allowed_integration_types as it's noted as supported in the below, and testing show it works. The document also shows EXTERNAL ACCESS as an acceptable integration type but that is still in preview in GCP and not something I've tested so left it out.

https://docs.snowflake.com/en/user-guide/account-replication-intro#replicated-objects
https://registry.terraform.io/providers/Snowflake-Labs/snowflake/latest/docs/resources/failover_group#allowed_integration_types

Added STORAGE INTEGRATIONS to allowed_integration_types as it's noted as supported in the below, and testing show it works.  The document also shows EXTERNAL ACCESS as an acceptable integration type but that is still in preview in GCP and not something I've tested so left it out. 

https://docs.snowflake.com/en/user-guide/account-replication-intro#replicated-objects
@sfc-gh-asawicki
Copy link
Collaborator

Hey @gdelia-pm. Thanks for the contribution!

I have a few remarks until we can merge it:

  1. We do not modify the doc md files directly. We update the resource schema (in this case, description) and generate the automatically (you can do this by running make pre-push). This change will be rewritten next time we generate the docs, so we have to follow this.
  2. I would like to see an automatic test that it works. The best way would be to modify one of the tests in failover_group_acceptance_test.go or failover_groups_integration_test.go. The first one is better because we recently unskipped these tests, and the ones in the second one are still awaiting.

Can you proceed with the requested changes, or should we queue them on our side?

@gdelia-pm
Copy link
Author

I'd prefer you queue them on your side. I've not done any of this before so suspect me trying would take way more time /effort than y'all.

@sfc-gh-asawicki
Copy link
Collaborator

That's fine! I will queue this on our side.

sfc-gh-jmichalak added a commit that referenced this pull request May 7, 2024
- Add missing types in SDK
- Update resource docs
- Update tests

<!-- Feel free to delete comments as you fill this in -->

<!-- summary of changes -->

## Test Plan
<!-- detail ways in which this PR has been tested or needs to be tested
-->
* [x] acceptance tests -- tested locally with Business Critical Edition
<!-- add more below if you think they are relevant -->
* [x] integration tests

## References
<!-- issues documentation links, etc  -->
https://docs.snowflake.com/en/sql-reference/sql/create-failover-group

Based on
#2518
@sfc-gh-jmichalak
Copy link
Collaborator

@gdelia-pm This was fixed in #2776

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants