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

[dagster-dbt] Upgrade dagster-dbt to be compatible with dbt 1.5.x #13907

Merged
merged 12 commits into from
Apr 27, 2023

Conversation

OwenKephart
Copy link
Contributor

@OwenKephart OwenKephart commented Apr 25, 2023

Summary & Motivation

Fixes: #13898

The changes:

  • --no-use-color is deprecated (--no-use-colors is allowed on all versions 1.0+)
  • The dbt selection string parsing logic relies on global state being set (I know, I know...), and the way that this global state is set has been changed (previously, they were attributes directly on the module, now they're all wrapped into a single GLOBAL_FLAGS object)
  • Nasty hack to deal with dbt-core shifting from argparse to click. We were using the argument parsing functionality to parse dbt Cloud job commands, so I found a somewhat similar entrypoint to work with.
  • Ensure that the current working directory when executing dbt commands is the project directory. This handles the issue linked below, where dbt now writes output files to the cwd rather than the project directory. This is not an issue if the cwd is the project directory ;)
  • Update some tests to be a bit more flexible to subtly different output formats (generally this is just testing stuff in the DbtCliOutput object, which we don't particularly expect or need to be very stable)
  • Some errors got reworded, so tests needed to be updated

In general, the select_unique_ids_from_manifest function access a lot of things it potentially shouldn't touch from the dbt internals, and there's no guarantee similar issues don't crop up in the future. The hope is that there is eventually a dbt-native python entrypoint for this functionality, at which point in time we can remove all this ugly stuff.

How I Tested These Changes

Added a dbt_15X tox env, which is pinned to the specific rcs that were available to me. Once the real dbt 1.5.x comes out, this should be unpinned

@OwenKephart
Copy link
Contributor Author

Currently blocked on: dbt-labs/dbt-core#7465, although should be possible to work around

@OwenKephart
Copy link
Contributor Author

Ok avoided the dbt-core bug for now, so that's no longer blocking.

@tha23rd
Copy link

tha23rd commented Apr 26, 2023

@OwenKephart

Not sure if this is beneficial for this PR at all, but wanted to put this on your radar (coming out in v1.5): https://docs.getdbt.com/reference/programmatic-invocations

I know dagster-dbt does a lot of stuff behind the scenes with the CLI, so this might help clean things up a bit

@rexledesma
Copy link
Contributor

@OwenKephart

Not sure if this is beneficial for this PR at all, but wanted to put this on your radar (coming out in v1.5): https://docs.getdbt.com/reference/programmatic-invocations

I know dagster-dbt does a lot of stuff behind the scenes with the CLI, so this might help clean things up a bit

@tha23rd Owen mentioned this internally to our team! Yeah this will be such a great QOL cleanup for our integration.

@rexledesma rexledesma force-pushed the owen/dbt-1.5 branch 2 times, most recently from 17fdbf8 to 66dbb73 Compare April 27, 2023 16:10
@rexledesma
Copy link
Contributor

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@rexledesma rexledesma merged commit 531d85c into master Apr 27, 2023
@rexledesma rexledesma deleted the owen/dbt-1.5 branch April 27, 2023 16:41
rexledesma added a commit that referenced this pull request Apr 27, 2023
…3907)

## Summary & Motivation

Fixes: #13898

The changes:

- `--no-use-color` is deprecated (`--no-use-colors` is allowed on all
versions 1.0+)
- The dbt selection string parsing logic relies on global state being
set (I know, I know...), and the way that this global state is set has
been changed (previously, they were attributes directly on the module,
now they're all wrapped into a single GLOBAL_FLAGS object)
- Nasty hack to deal with dbt-core shifting from `argparse` to `click`.
We were using the argument parsing functionality to parse dbt Cloud job
commands, so I found a somewhat similar entrypoint to work with.
- Ensure that the current working directory when executing dbt commands
is the project directory. This handles the issue linked below, where dbt
now writes output files to the cwd rather than the project directory.
This is not an issue if the cwd *is* the project directory ;)
- Update some tests to be a bit more flexible to subtly different output
formats (generally this is just testing stuff in the DbtCliOutput
object, which we don't particularly expect or need to be very stable)
- Some errors got reworded, so tests needed to be updated

In general, the `select_unique_ids_from_manifest` function access a lot
of things it potentially shouldn't touch from the dbt internals, and
there's no guarantee similar issues don't crop up in the future. The
hope is that there is eventually a dbt-native python entrypoint for this
functionality, at which point in time we can remove all this ugly stuff.

## How I Tested These Changes

Added a dbt_15X tox env, which is pinned to the specific rcs that were
available to me. Once the real `dbt 1.5.x` comes out, this should be
unpinned

---------

Co-authored-by: Rex Ledesma <rex@elementl.com>
takeru911 pushed a commit to takeru911/dagster that referenced this pull request Apr 29, 2023
…gster-io#13907)

## Summary & Motivation

Fixes: dagster-io#13898

The changes:

- `--no-use-color` is deprecated (`--no-use-colors` is allowed on all
versions 1.0+)
- The dbt selection string parsing logic relies on global state being
set (I know, I know...), and the way that this global state is set has
been changed (previously, they were attributes directly on the module,
now they're all wrapped into a single GLOBAL_FLAGS object)
- Nasty hack to deal with dbt-core shifting from `argparse` to `click`.
We were using the argument parsing functionality to parse dbt Cloud job
commands, so I found a somewhat similar entrypoint to work with.
- Ensure that the current working directory when executing dbt commands
is the project directory. This handles the issue linked below, where dbt
now writes output files to the cwd rather than the project directory.
This is not an issue if the cwd *is* the project directory ;)
- Update some tests to be a bit more flexible to subtly different output
formats (generally this is just testing stuff in the DbtCliOutput
object, which we don't particularly expect or need to be very stable)
- Some errors got reworded, so tests needed to be updated

In general, the `select_unique_ids_from_manifest` function access a lot
of things it potentially shouldn't touch from the dbt internals, and
there's no guarantee similar issues don't crop up in the future. The
hope is that there is eventually a dbt-native python entrypoint for this
functionality, at which point in time we can remove all this ugly stuff.

## How I Tested These Changes

Added a dbt_15X tox env, which is pinned to the specific rcs that were
available to me. Once the real `dbt 1.5.x` comes out, this should be
unpinned

---------

Co-authored-by: Rex Ledesma <rex@elementl.com>
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.

Latest version of dagster-dbt (v1.3.1) incompatible with dbt v1.5
3 participants