Skip to content

Conversation

@Vitor-Avila
Copy link
Contributor

Currently the dbt Cloud sync command expects that the DB connection in Preset is created based on the DB name in dataset (the actual database name). Keeping these matched is non intuitive, specially now that Superset supports multi-catalog (meaning that a single DB connection in Superset can interact with n catalogs).

This PR introduces the ability to specify a --database-id=x in the dbt Cloud command to perform the mapping.

Copy link
Member

@betodealmeida betodealmeida left a comment

Choose a reason for hiding this comment

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

Looks good, but wouldn't it be better to use the UUID instead? (I'm not sure if it's easy.)

@Vitor-Avila
Copy link
Contributor Author

Looks good, but wouldn't it be better to use the UUID instead? (I'm not sure if it's easy.)

I think the uuid would be easier if the user is doing both the native and dbt sync (as they would have the YAML handy and that's a universal ID even across Workspaces). But for the dbt sync specifically, I think the id or the database_name would be easier (considering they don't have the YAML and the uuid is not exposed on a lot of endpoints).

If you think it makes more sense I could update this to be --database-name instead (or even add both and make them exclusive (you can use one OR the other at a time)).

@betodealmeida
Copy link
Member

I think the uuid would be easier if the user is doing both the native and dbt sync (as they would have the YAML handy and that's a universal ID even across Workspaces). But for the dbt sync specifically, I think the id or the database_name would be easier (considering they don't have the YAML and the uuid is not exposed on a lot of endpoints).

Ah, makes sense!

@betodealmeida
Copy link
Member

betodealmeida commented Jan 28, 2025

Nice! Double approved! ✅ ✅

@Vitor-Avila Vitor-Avila merged commit f9ade14 into main Jan 28, 2025
5 checks passed
@Vitor-Avila Vitor-Avila deleted the feat/specify-db-dbt-cloud branch January 28, 2025 20:20
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