Repository navigation
Conversation
The fromyaml builtin wrapped the parsed document in dict(), so YAML whose top level is not a mapping was mishandled: a block-style list raised an uncaught ValueError, and a flow-style list or a scalar was silently returned as None. dbt returns the parsed value as is. sqlmesh.utils.yaml.load already returns plain Python objects, so return its result directly. Mappings are unaffected. Fixes SQLMesh#6137 Signed-off-by: JanakiRamChittineni <341781521+raams95@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #6137.
In dbt projects, SQLMesh's
fromyamlbuiltin wrapped the parsed document indict(...), so YAML whose top level isn't a mapping came back wrong:- good\n- bad) raised an uncaughtValueError, which failed rendering.[1, 2]) or a scalar (42) quietly returnedNone, because theTypeErrorfromdict()is caught anddefaultis returned.dbt's
fromyamlreturnsyaml.safe_load(value)unchanged.sqlmesh.utils.yaml.loadalready returns plain Python objects (dict, list, int, ...), so this change drops thedict(...)wrapper and returns the parsed value directly. Mappings give the same result as before, and invalid YAML still returnsdefault.Test Plan
test_yamlintests/dbt/test_transformation.pywith a flow-style list, a block-style list (built with{% set %}, like the example in dbt's docs) and a scalar. Onmainthe test fails: it rendersNonefor[1, 2], and the block-style list raisesValueError: dictionary update sequence element #0 has length 4; 2 is required. With this change it passes.pytest tests/dbt(slow tests included) on Windows: 296 passed, 2 skipped. One test,test_load_custom_materialisations, errored in the parallel run, but it passes on its own both on this branch and onmain. To run thesushi_testbased tests locally, I replaced thesushi_test/dbt_packagessymlinks with directory junctions, since git checks them out as plain files on Windows.ruff checkandruff format --checkare clean on the changed files.mypy sqlmesh/dbt/builtin.pyonly reports the errors my local environment also gives onmain(missingsqlmesh._version,signal.SIGUSR1).Checklist
make styleand fixed any issues (ran ruff + mypy on the changed files only; on Windows, mypy only shows the same environment errors as onmain)make fast-test) (rantests/dbton Windows and compared againstmain; I didn't run the full suite)git commit -s) per the DCO