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] fixtures in fixtures/subfolders throwing parsing error #9570

Closed
2 tasks done
Tracked by #8283
graciegoheen opened this issue Feb 13, 2024 · 2 comments · Fixed by #9714
Closed
2 tasks done
Tracked by #8283

[Bug] fixtures in fixtures/subfolders throwing parsing error #9570

graciegoheen opened this issue Feb 13, 2024 · 2 comments · Fixed by #9714
Assignees
Labels
bug Something isn't working High Severity bug with significant impact that should be resolved in a reasonable timeframe pre-release Bug not yet in a stable release unit tests Issues related to built-in dbt unit testing functionality
Milestone

Comments

@graciegoheen
Copy link
Contributor

graciegoheen commented Feb 13, 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

I have a unit test definition using csv fixtures:

unit_tests:  
  - name: test_cloud_account_mappings
    model: int_cloud_account_mappings__stepped 
    given: 
      - input: ref('int_account_domains__ranked')
        format: csv
        fixture: cloud_account_mappings_mock_input
    expect: 
      format: csv
      fixture: cloud_account_mappings_mock_output

Everything works as expected when I nest my fixture files directly in the fixtures folder:

  • fixtures/cloud_account_mappings_mock_output.csv
  • fixtures/cloud_account_mappings_mock_input.csv

If i instead nest my fixtures in a subfolder, I get a parsing error:

  • fixtures/cloud_account_mappings/cloud_account_mappings_mock_output.csv
  • fixtures/cloud_account_mappings/cloud_account_mappings_mock_input.csv
23:21:34 Unable to do partial parsing because an error occurred. Switching to full reparse.
23:21:34 Encountered an error:
not enough values to unpack (expected 3, got 1)
23:21:34 Traceback (most recent call last):
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/parser/manifest.py", line 392, in load
    project_parser_files = self.partial_parser.get_parsing_files()
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/parser/partial.py", line 191, in get_parsing_files
    self.delete_from_saved(file_id)
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/parser/partial.py", line 288, in delete_from_saved
    self.delete_fixture_node(saved_source_file)
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/parser/partial.py", line 609, in delete_fixture_node
    unit_test = self.saved_manifest.unit_tests.pop(unique_id)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
KeyError: 'unit_test.fishtown_internal_analytics.int_cloud_account_mappings__stepped.test_cloud_account_mappings'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/cli/requires.py", line 109, in wrapper
    result, success = func(*args, **kwargs)
                      ^^^^^^^^^^^^^^^^^^^^^
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/cli/requires.py", line 94, in wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/cli/requires.py", line 187, in wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/cli/requires.py", line 216, in wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/cli/requires.py", line 263, in wrapper
    return func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/cli/requires.py", line 289, in wrapper
    ctx.obj["manifest"] = parse_manifest(
                          ^^^^^^^^^^^^^^^
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/parser/manifest.py", line 1846, in parse_manifest
    manifest = ManifestLoader.get_full_manifest(
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/parser/manifest.py", line 324, in get_full_manifest
    manifest = loader.load()
               ^^^^^^^^^^^^^
  File "/venv/dbt-versionless/lib/python3.11/site-packages/dbt/parser/manifest.py", line 407, in load
    (_, line, method) = formatted_lines[-3].split(", ")
    ^^^^^^^^^^^^^^^^^
ValueError: not enough values to unpack (expected 3, got 1)
23:21:15 not enough values to unpack (expected 3, got 1)

Expected Behavior

I should be able to nest my fixture files in subfolders for better repo organization

Which database adapter are you using with dbt?

snowflake

Additional Context

No response

@graciegoheen graciegoheen added bug Something isn't working pre-release Bug not yet in a stable release labels Feb 13, 2024
@graciegoheen graciegoheen added this to the v1.8 milestone Feb 13, 2024
@graciegoheen graciegoheen added the High Severity bug with significant impact that should be resolved in a reasonable timeframe label Feb 13, 2024
slothkong added a commit to slothkong/dbt-core that referenced this issue Feb 29, 2024
slothkong added a commit to slothkong/dbt-core that referenced this issue Feb 29, 2024
slothkong added a commit to slothkong/dbt-core that referenced this issue Mar 6, 2024
* Transform skip_parsing (private variable of ManifestLoader.load()) into instance-attribute of ManifestLoader(), with default value False
  (to enable splitting of ManifestLoader.load())

* Split ManifestLoader.load(), to extract operation of PartialParsing into new method called ManifestLoader.safe_update_project_parser_files_partially()
  (to simplify both cognitive complexity in the codebase and mocking in unittestest)

* Add "ignore" type-comments in new ManifestLoader.safe_update_project_parser_files_partially()
  (to silence mypy warnings regarding instance-attributes which can be initialized as None or as something else, e.g. self.saved_manifest)[1]

[1] Although I wanted avoid "ignore" type-comments, it seems like addressing these mypy warnings in a stricter sense requires technical alignment and broader code changes.
	For example, might need to initialize self.saved_manifest as Manifest, instead of Optional[Manifest], so that PartialParsing gets inputs with type it currently expects.
	... perhaps too far beyond the scope of this fix?
gshank pushed a commit that referenced this issue Mar 13, 2024
…9714)

* [#9570] Fix fixtures in fixtures/subfolders throwing parsing error

* Fast-forward imports to match upstream

* Re-introduce doc strings on traceback info handling

* [#9570] Changelog update for fix of fixtures in fixtures/subfolders throwing parsing error

* [#9570] Improve testability and coverage for partial parsing

* Transform skip_parsing (private variable of ManifestLoader.load()) into instance-attribute of ManifestLoader(), with default value False
  (to enable splitting of ManifestLoader.load())

* Split ManifestLoader.load(), to extract operation of PartialParsing into new method called ManifestLoader.safe_update_project_parser_files_partially()
  (to simplify both cognitive complexity in the codebase and mocking in unittestest)

* Add "ignore" type-comments in new ManifestLoader.safe_update_project_parser_files_partially()
  (to silence mypy warnings regarding instance-attributes which can be initialized as None or as something else, e.g. self.saved_manifest)[1]

[1] Although I wanted avoid "ignore" type-comments, it seems like addressing these mypy warnings in a stricter sense requires technical alignment and broader code changes.
	For example, might need to initialize self.saved_manifest as Manifest, instead of Optional[Manifest], so that PartialParsing gets inputs with type it currently expects.
	... perhaps too far beyond the scope of this fix?
@emmyoop emmyoop reopened this Mar 15, 2024
@gshank
Copy link
Contributor

gshank commented Mar 15, 2024

A pull request, #9714, fixed the secondary exception reported in this ticket, but not the primary error.

@emmyoop
Copy link
Member

emmyoop commented Mar 15, 2024

Upon further investigation, this no longer seems to be an issue. Closing.

@emmyoop emmyoop closed this as completed Mar 15, 2024
@dbeatty10 dbeatty10 added the unit tests Issues related to built-in dbt unit testing functionality label Sep 20, 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 High Severity bug with significant impact that should be resolved in a reasonable timeframe pre-release Bug not yet in a stable release unit tests Issues related to built-in dbt unit testing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants