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

[Bug] dbt 1.8 does not work correctly with sqlfluff #10176

Closed
2 tasks done
kzosabe opened this issue May 19, 2024 · 2 comments
Closed
2 tasks done

[Bug] dbt 1.8 does not work correctly with sqlfluff #10176

kzosabe opened this issue May 19, 2024 · 2 comments
Labels
bug Something isn't working wontfix Not a bug or out of scope for dbt-core

Comments

@kzosabe
Copy link

kzosabe commented May 19, 2024

Is this a new bug in dbt-core?

  • I believe this is a new bug in dbt-core
  • I have searched the existing issues, and I could not find an existing issue for this bug

Current Behavior

dbt 1.8 does not work correctly with sqlfluff

Expected Behavior

dbt 1.8 works correctly with sqlfluff

Steps To Reproduce

Check out this PR and run sqlfluff lint or unit tests with dbt 1.8
sqlfluff/sqlfluff#5882

Relevant log output

Some error stack traces described in additional context

Environment

- OS: Mac / Linux
- Python: 3.8 or later
- dbt: 1.8.0

Which database adapter are you using with dbt?

No response

Additional Context

I'm a contributor of sqlfluff and am working on making sqlfluff compatible with dbt 1.8.
sqlfluff/sqlfluff#5877

Some incompatibilities have been fixed in this PR, but I found some problems that don't seem to be resolvable by sqlfluff alone.
sqlfluff/sqlfluff#5882

The sqlfluff templater for dbt (sqlfluff-templater-dbt) checks the user's dbt configuration via dbt's ManifestLoader . When sqlfluff calls ManifestLoader.get_full_manifest(self.dbt_config) in dbt 1.8, internal errors occur on dbt's side.

I succeeded in suppressing the errors by editing dbt's code directly, but I'm unsure if these changes are appropriate. I think it would be best if we could resolve the errors inside sqlfluff, for example, by adding the necessary configurations.

Please advise if there are any ways to resolve these errors on sqlfluff's side, or if better fixes should be made in dbt-core.

problem details

It seems there are two problems that the adapter itself can't solve. I confirmed that direct edits to dbt's code in my local environment to fix the two issues described below make it work.

1. get_invocation_context

After the code fixes in above PR, if we run sqlfluff lint with dbt 1.8, it fails like this.

> sqlfluff lint example.sql
...
  File "/Users/kzosabe/ws/dev/oss/python/sqlfluff/plugins/sqlfluff-templater-dbt/sqlfluff_templater_dbt/templater.py", line 252, in dbt_manifest
    self.dbt_manifest = ManifestLoader.get_full_manifest(self.dbt_config)
  File "/Users/kzosabe/ws/dev/oss/python/sqlfluff/.venv/lib/python3.8/site-packages/dbt/parser/manifest.py", line 330, in get_full_manifest
    manifest = loader.load()
  File "/Users/kzosabe/ws/dev/oss/python/sqlfluff/.venv/lib/python3.8/site-packages/dbt/parser/manifest.py", line 384, in load
    project_parser_files = self.safe_update_project_parser_files_partially(
  File "/Users/kzosabe/ws/dev/oss/python/sqlfluff/.venv/lib/python3.8/site-packages/dbt/parser/manifest.py", line 536, in safe_update_project_parser_files_partially
    self.partial_parser = PartialParsing(self.saved_manifest, self.manifest.files)  # type: ignore[arg-type]
  File "/Users/kzosabe/ws/dev/oss/python/sqlfluff/.venv/lib/python3.8/site-packages/dbt/parser/partial.py", line 86, in __init__
    self.build_file_diff()
  File "/Users/kzosabe/ws/dev/oss/python/sqlfluff/.venv/lib/python3.8/site-packages/dbt/parser/partial.py", line 165, in build_file_diff
    if get_invocation_context().env.get("DBT_PP_TEST"):
  File "/Users/kzosabe/ws/dev/oss/python/sqlfluff/.venv/lib/python3.8/site-packages/dbt_common/context.py", line 53, in get_invocation_context
    ctx = invocation_var.get()
LookupError: <ContextVar name='DBT_INVOCATION_CONTEXT_VAR' at 0x1064ffa90>
        if get_invocation_context().env.get("DBT_PP_TEST"):
            fire_event(event, level=EventLevel.INFO)

It seems that calling get_invocation_context() inside the sqlfluff context isn't working properly. This line was introduced below. If we revert this line to if os.environ.get("DBT_PP_TEST"):, no errors occur.

ef03ea2#diff-2fd42940aaee0e200a257863155b747faf735f6b49ae49ac9229c17be33db82aR165

My question is, are there any ways to make get_invocation_context() work properly outside of dbt's normal context, such as within sqlfluff?

2. REQUIRE_RESOURCE_NAMES_WITHOUT_SPACES

If we fixed 1. and run sqlfluff lint again, it fails like this.

> sqlfluff lint example.sql
...
  File "/Users/kzosabe/ws/dev/oss/python/sqlfluff/plugins/sqlfluff-templater-dbt/sqlfluff_templater_dbt/templater.py", line 252, in dbt_manifest
    self.dbt_manifest = ManifestLoader.get_full_manifest(self.dbt_config)
  File "/Users/kzosabe/ws/dev/oss/python/sqlfluff/.venv/lib/python3.8/site-packages/dbt/parser/manifest.py", line 330, in get_full_manifest
    manifest = loader.load()
  File "/Users/kzosabe/ws/dev/oss/python/sqlfluff/.venv/lib/python3.8/site-packages/dbt/parser/manifest.py", line 528, in load
    self.check_for_spaces_in_resource_names()
  File "/Users/kzosabe/ws/dev/oss/python/sqlfluff/.venv/lib/python3.8/site-packages/dbt/parser/manifest.py", line 641, in check_for_spaces_in_resource_names
    if self.root_project.args.REQUIRE_RESOURCE_NAMES_WITHOUT_SPACES
AttributeError: 'DbtConfigArgs' object has no attribute 'REQUIRE_RESOURCE_NAMES_WITHOUT_SPACES'
        level = (
            EventLevel.ERROR
            if self.root_project.args.REQUIRE_RESOURCE_NAMES_WITHOUT_SPACES
            else EventLevel.WARN
        )

This line was introduced below.
a36057d#diff-a25a77362db78f4ddc0675bcbd4775e158631076c9918ce5b64cfcd9f6db6bceR641

If we make it like below, no errors occur.

        level = (
            EventLevel.ERROR
            if (
                hasattr(self.root_project.args, "REQUIRE_RESOURCE_NAMES_WITHOUT_SPACES") and
                self.root_project.args.REQUIRE_RESOURCE_NAMES_WITHOUT_SPACES
            )
            else EventLevel.WARN
        )

I'm not sure if this change is the best approach. It's possible that they expected a default value for REQUIRE_RESOURCE_NAMES_WITHOUT_SPACES and it was missed during implementation. I'm not familiar with dbt's codebase, so I couldn't confirm that.

My question is, is there any way for sqlfluff to set values for REQUIRE_RESOURCE_NAMES_WITHOUT_SPACES, or for dbt itself to set default values for REQUIRE_RESOURCE_NAMES_WITHOUT_SPACES ?

@kzosabe kzosabe added bug Something isn't working triage labels May 19, 2024
@kzosabe
Copy link
Author

kzosabe commented May 21, 2024

These issues were resolved in sqlfluff/sqlfluff#5892 and no longer need to be answered.
Sorry for raising the issue prematurely, closed.

@kzosabe kzosabe closed this as completed May 21, 2024
@jtcohen6
Copy link
Contributor

Thank you so much for raising the initial issue @kzosabe, and for the update!

@jtcohen6 jtcohen6 removed the triage label May 21, 2024
@dbeatty10 dbeatty10 closed this as not planned Won't fix, can't repro, duplicate, stale May 21, 2024
@dbeatty10 dbeatty10 added the wontfix Not a bug or out of scope for dbt-core label May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working wontfix Not a bug or out of scope for dbt-core
Projects
None yet
Development

No branches or pull requests

3 participants