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

chore(dbt sync): Deprecate using the dbt_project file to trigger a dbt sync #299

Merged
merged 2 commits into from
Jun 3, 2024

Conversation

Vitor-Avila
Copy link
Contributor

It's possible to trigger the dbt Core sync specifying the path to either the manifest.json file or to the dbt_project.yml file. The former would currently display a DeprecationWarning.

dbt deprecated the target-path as a config in the dbt_project.yml (which is used when triggering the command with this file) so this PR is moving the deprecated warning to when syncing with the path to dbt_project.yml file specified. For simplicity, I would recommend dropping support entirely in a future release and relying solely in the manifest.json file.

This PR also moves away from using warnings.warn() as it breaks logging (not a concern when using the preset-cli as a standalone CLI, but could be impactful when using it as a dependency and relying on its methods).

@@ -160,17 +157,18 @@ def dbt_core( # pylint: disable=too-many-arguments, too-many-branches, too-many
"The managed externally feature was only introduced in Superset v1.5."
"Make sure you are running a compatible version."
)
log_warning(warn_message, UserWarning)
_logger.debug(warn_message)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to debug as it might make more sense (given it might not be a concern).

Comment on lines +167 to +169
"Passing the dbt_project.yml file is deprecated and "
"will be removed in a future version. "
"Please pass the manifest.json file instead."
Copy link
Member

Choose a reason for hiding this comment

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

lol

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hoping to have a single supported method soon.

@Vitor-Avila Vitor-Avila merged commit ae99f52 into main Jun 3, 2024
5 checks passed
@Vitor-Avila Vitor-Avila deleted the deprecate-sync-with-project branch June 3, 2024 23:13
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