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

feat: Data source for list databases #861

Merged
merged 5 commits into from
Feb 22, 2022
Merged

Conversation

jmrobles
Copy link
Contributor

Add the ability to read the current databases in Snowflake (as far as the databases that the role used has the USAGE privilege on those databases).

Test Plan

It's easy to test. Just run and retrieve the Snowflake Account's databases

[X] Acceptance test

References

Feature Request: #859

@alldoami alldoami changed the title FEAT Data source for list databases feat: Data source for list databases Feb 15, 2022
@alldoami
Copy link
Contributor

Would it be possible to instead create a snowflake_database data source instead?

@jmrobles
Copy link
Contributor Author

Sure! why not? The reason to use it plural is due to avoid some confusion respect the resource name "snowflake_database", but if you consider there is no problem with that I can update the PR with the data source in singular :)

@alldoami
Copy link
Contributor

@jmrobles ya because we use data before the name I don't think there should be confusion! I think it'd be more standard to import as a singular resource than a list.

@alldoami
Copy link
Contributor

hmmm @jmrobles sorry I don't think I was clear 😅 Instead of outputting a list of databases, do you think it's possible to create the data source to just read one database from a specified database_name? If people want to use this data source and want a list of all databases, I think they'd have to specify a string list of database names in a locals variable and then use a for_each to create a data source for each one. I think this use case of getting a list of databases is a bit too niche of a situation.

@jmrobles
Copy link
Contributor Author

Ah, ok I understand you.

The motivation of this PR is because in a project that I'm working on, we need to create a user account per database.

The idea is that automatically those user will be maintained without the need of declare in locals or vars section.

@alldoami
Copy link
Contributor

@jmrobles ah ok, then let's change it back to plural. Would be great if you could create a new data source with the singular form so that people can use it as a singular data source instead of a list.

name = "DEMO_DB"
}

resource "snowflake_database" "backup" {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can remove this resource, you can just keep the data source as the example. Same for the one below

@alldoami
Copy link
Contributor

/ok-to-test sha=abe9aca

@github-actions
Copy link

Integration tests failure for abe9aca

@jmrobles
Copy link
Contributor Author

I needed to write a custom TestStep check function.
Tested against Snowflake (own account).

@alldoami
Copy link
Contributor

/ok-to-test sha=be94510

@github-actions
Copy link

Integration tests success for be94510

@alldoami
Copy link
Contributor

Could you run make docs?

@jmrobles
Copy link
Contributor Author

Sure! Done, sorry, I forgot to re-run it.

@alldoami
Copy link
Contributor

/ok-to-test sha=7145a89

@github-actions
Copy link

Integration tests success for 7145a89

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.

None yet

2 participants