Fix code to work with mashumaro 3.15 #11051
Conversation
|
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11051 +/- ##
=======================================
Coverage 88.86% 88.87%
=======================================
Files 187 187
Lines 24028 24004 -24
=======================================
- Hits 21353 21333 -20
+ Misses 2675 2671 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
gshank
changed the title
|
Also see dbt-labs/dbt-common#228 |
| own_config = self.get_node_project(project_name) | ||
| ) -> Dict[str, Any]: | ||
| """This method takes resource configs from the project, the model (if applicable), | ||
| and the patch, and combines them into one config dictionary.""" |
There was a problem hiding this comment.
this whole method is so much more readable now, thank you ✨
| finalized = config.finalize_and_validate() | ||
| return finalized.to_dict(omit_none=True) | ||
| config_cls.validate(config_dict) | ||
| config_obj = config_cls.from_dict(config_dict) |
There was a problem hiding this comment.
I'm noticing we are no longer omitting None values during construction here.
Is the implication of that that even None configurations will make their way onto the parsed node representation + get serialized out? I don't think this is a huge deal, just trying to wrap my head around any potential downsides of the change.
There was a problem hiding this comment.
The config_dict is the merged dicts from all the various sources, and the method actually returns a config obj. It doesn't really matter whether you've omitted the None fields when constructing an object. This is called in build_config_dict, which converts it to a dictionary with omit_none, so it should actually be the same dictionary that was being returned before.
| for hook in hooks: | ||
| get_rendered(hook.sql, context, parsed_node, capture_macros=True) | ||
|
|
||
| def initial_config(self, fqn: List[str]) -> ContextConfig: | ||
| config_version = min([self.project.config_version, self.root_project.config_version]) | ||
| if config_version == 2: |
There was a problem hiding this comment.
Is it safe to remove this check entirely at this point?
There was a problem hiding this comment.
The config_version == 2? Yeah, we haven't required that or checked in at least a year and a half. I think pre-version-2 was way back in... version 14? Before I started anyway.
Resolves #11044
Problem
Mashumaro 3.15 caused some breakages in our code.
Solution
In a couple of cases, Unions had to be adjusted to de-serialize and serialize in the right order. Most of the effort went into refactoring the update node config code so that "from_dict" was not performed until after validation, so that config values were not converted to valid forms prior to being validated. In general the pattern is that all sources of configs should be combined as dictionaries, then validated, then used to construct a valid config object.
dbt-common BaseConfig.update_from method had to be converted to a class method and the "from_dict" removed.
Since there were already a bunch of changes in core/dbt/context/context_config.py, I also took the opportunity to rename ContextConfig to ConfigBuilder, and replace variables and parameters using it in the rest of the parsing codebase.
Since the hooks needed to be fixed prior to validation in generate_node_config, I moved the hook fixing method to context_config.py and removed it from parser/base.py.
Checklist