Fix: Sort nested AttributeDict's to prevent visual diff#5397
Conversation
| return macro_references, variables | ||
|
|
||
|
|
||
| def sort_dict_recursive( |
There was a problem hiding this comment.
I couldnt find an existing util function that would recurse into values and sort those too, let me know if it exists and i'm just blind
There was a problem hiding this comment.
@eakmanrq did we end up removing the utility you implemented for python_env sorting?
There was a problem hiding this comment.
do you mean _dict_sort or sort_python_env? i'm not sure either of these would achieve the same, unless there is another one
There was a problem hiding this comment.
Yeah sorry I'm bad with Github notification. I originally added a recrusive dict sort but rolled that back in this PR and just sort the top level: #4984
georgesittas
left a comment
georgesittas
left a comment
There was a problem hiding this comment.
@erindru why is the inner dictionary order non-deterministic?
| return macro_references, variables | ||
|
|
||
|
|
||
| def sort_dict_recursive( |
There was a problem hiding this comment.
@eakmanrq did we end up removing the utility you implemented for python_env sorting?
We like to use sets. It's "deterministic" until the cache is busted and the project is re-loaded from disk. I figured that it was easier to fix it once on output rather than trying to fix every call site now and also again in future when we inevitably make this mistake again and again |
4 participants
Prior to this, on dbt projects, if a model had a change, often the text diff would show a diff for
refseven though those didn't change at all. In this example, all I did was add astamp, but the text diff shows a diff forrefs:The cause is that while the keys ("refs", "sources", "vars") were sorted - the values weren't. So calling
model.render_definition()on the Snapshot in state, callingmodel.render_definition()the local Snapshot and comparing the output could result in a text diff, even though nothing had actually changed, because the values are output in a non-deterministic order.After this PR, we no longer see the extra "change" and the text diff focuses on what actually got changed:
Note: if you try to repro this locally, you need to run
sqlmesh cleanto clear the cache inbetween plans otherwise a specific order gets cached and you never see the erroneous diff