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

docs: Clarify account identifier formats #1398

Merged
merged 2 commits into from
Dec 7, 2022
Merged

docs: Clarify account identifier formats #1398

merged 2 commits into from
Dec 7, 2022

Conversation

cappadona
Copy link
Contributor

@cappadona cappadona commented Dec 6, 2022 •
edited
Loading

While reviewing the provider docs we noticed an inconsistency regarding required keys for the provider configuration.

Example configuration identifies region as required

Screen Shot 2022-11-23 at 5 01 55 PM

Schema documentation identifies region as optional

Screen Shot 2022-11-23 at 5 16 56 PM

This updated documentation consistently identifies region as optional, but indicates it is needed if using the legacy format for the account identifier.

Also includes minor edits for consistent language around sourcing values from environment variables.

Test Plan

  • acceptance tests

References

@cappadona
Clarify documentation around account identifier
2c962c4 
* two formats
  * preferred: `<org_name>-<account_name>`
  * legacy: `<account_locator>.<cloud_region_id>.<cloud>`
* `region` is required only if using legacy format

https://docs.snowflake.com/en/user-guide/admin-account-identifier.html
@cappadona
Consistent language for sourcing env vars
11b4328
@cappadona cappadona mentioned this pull request Dec 6, 2022
Closed
@sfc-gh-swinkler
Copy link
Collaborator

sfc-gh-swinkler commented Dec 7, 2022 •
edited
Loading

@cappadona this looks good. Clarity with documentation is always appreciated.

Could you please run make docs so that the check-docs step can pass?

edit: i'll take care of this for now, since we need to get the new release out. Thanks for your contribution!

@sfc-gh-swinkler sfc-gh-swinkler merged commit 7a8cf75 into Snowflake-Labs:main Dec 7, 2022
@cappadona
Copy link
Contributor Author

Thanks @sfc-gh-swinkler! 🙏🏼

@cappadona
Copy link
Contributor Author

@sfc-gh-swinkler Unfortunately, it looks like the changes from this PR were (unintentionally?) reverted in #1403 and therefore not included in the latest v0.53.0 release

@sfc-gh-swinkler
Copy link
Collaborator

@cappadona sorry about that, an oversight on my part. i am planning to do a 0.53.1 release later today to update some dependencies, so i can include this back in.

@cappadona
Copy link
Contributor Author

No worries, @sfc-gh-swinkler. Let me know if you need anything from me. Thank you.

@sfc-gh-swinkler
Copy link
Collaborator

Hello @cappadona , so i have a PR to add this back in. #1409. The reason this happened was because the changes were made to the docs folder, which is a generated folder and gets overridden whenever make docs is run.

@cappadona
Copy link
Contributor Author

Ah...my bad. I missed that key detail. Thanks for pointing this out and making the changes. Apologies for the hassle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sfc-gh-swinkler sfc-gh-swinkler sfc-gh-swinkler approved these changes

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

Successfully merging this pull request may close these issues.
2 participants
@cappadona @sfc-gh-swinkler