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

[CT-663] YAML anchor expansion failure in 1.2.0a1 / main #5268

Closed
jtcohen6 opened this issue May 18, 2022 · 3 comments
Closed

[CT-663] YAML anchor expansion failure in 1.2.0a1 / main #5268

jtcohen6 opened this issue May 18, 2022 · 3 comments
Labels
bug Something isn't working
Milestone

Comments

@jtcohen6
Copy link
Contributor

Proactively identified regression in 1.2.0a1. If I had to guess, it might be related to #5146?

Here's a profile that uses YAML anchors to DRY up duplicated code. This is valid YAML, per "online YAML parser":

# profiles.yml
garage-postgres:
  outputs:
    dev: &garage-pg
      database: jerco
      host: localhost
      pass: nopass
      port: 5432
      schema: dbt_jcohen
      threads: 5
      type: postgres
      user: jerco
    prod:
      <<: *garage-pg
      schema: analytics
  target: dev

When running v1.1.0, dbt is perfectly happy to expand the YAML anchor:

$ dbt debug
11:02:25  Running with dbt=1.1.0
dbt version: 1.1.0
python version: 3.9.12
python path: /Users/jerco/dev/scratch/testy/env/bin/python3.9
os info: macOS-12.3.1-x86_64-i386-64bit
Using profiles.yml file at /Users/jerco/.dbt/profiles.yml
Using dbt_project.yml file at /Users/jerco/dev/scratch/testy/dbt_project.yml

Configuration:
  profiles.yml file [OK found and valid]
  dbt_project.yml file [OK found and valid]

Required dependencies:
 - git [OK found]

Connection:
  host: localhost
  port: 5432
  user: jerco
  database: jerco
  schema: dbt_jcohen
  search_path: None
  keepalives_idle: 0
  sslmode: None
  Connection test: [OK connection ok]

All checks passed!

Running dbt-core installed from main, it's not:

$ dbt debug
11:02:38  Running with dbt=1.2.0-a1
dbt version: 1.2.0-a1
python version: 3.9.12
python path: /Users/jerco/dev/product/dbt-core/env/bin/python3.9
os info: macOS-12.3.1-x86_64-i386-64bit
Using profiles.yml file at /Users/jerco/.dbt/profiles.yml
Using dbt_project.yml file at /Users/jerco/dev/scratch/testy/dbt_project.yml

11:02:39  Encountered an error:
Runtime Error

  dbt encountered an error while trying to read your profiles.yml file.

  Runtime Error
    Syntax error near line 55
    ------------------------------
    52 |       type: postgres
    53 |       user: jerco
    54 |     prod:
    55 |       <<: *garage-pg
    56 |       schema: analytics
    57 |   target: dev
    58 | sandbox-redshift:

    Raw Error:
    ------------------------------
    could not determine a constructor for the tag 'tag:yaml.org,2002:merge'
      in "<unicode string>", line 55, column 7
@jtcohen6 jtcohen6 added bug Something isn't working regression Team:Language labels May 18, 2022
@github-actions github-actions bot changed the title YAML anchor expansion failure in 1.2.0a1 / main [CT-663] YAML anchor expansion failure in 1.2.0a1 / main May 18, 2022
@jtcohen6
Copy link
Contributor Author

jtcohen6 commented May 18, 2022

On second thought, this shouldn't actually be labeled regression, since this change has yet to be included in any actual (pre)releases. But we definitely need a test case for this sort of thing, since folks do make use of yaml anchors! (e.g.)

@jtcohen6
Copy link
Contributor Author

jtcohen6 commented Jun 6, 2022

I'm inclined to think this is an issue with using UniqueKeyLoader instead of SafeLoader. If I switch safe_load back to using SafeLoader instead:

def safe_load(contents) -> Optional[Dict[str, Any]]:
return yaml.load(contents, Loader=UniqueKeyLoader)

Then everything works fine.

In order to resolve this issue, there are two options, and we might want to pursue both of them:

  • Adjust the logic in UniqueKeyLoader.construct_mapping, which we've rolled ourselves, to handle these cases
  • In all cases, fall back to using standard SafeLoader if UniqueKeyLoader encounters ANY error. (In some cases, we do want to raise that error as a warning)

No matter the approach taken, we MUST add automated test cases reflecting the end-user behavior / reproduction cases in both of these issues, since this is real code that users have out in the wild.

@jtcohen6
Copy link
Contributor Author

Resolved by #5403. We ended up reverting #5146

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant