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

Support for dbt core changes to support mashumaro 3.15 #228

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
46 changes: 22 additions & 24 deletions dbt_common/contracts/config/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,28 @@ def same_contents(cls, unrendered: Dict[str, Any], other: Dict[str, Any]) -> boo
"object": ["snapshot_meta_column_names"],
}

@classmethod
def update_from(
cls, dct: Dict[str, Any], data: Dict[str, Any], adapter_config_cls: Type[BaseConfig]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

naming nit: is there naming here that would better signify the difference between dct and data?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have any suggestions? orig_dict, new_dict?

) -> Dict[str, Any]:
"""Update and validate config given a dict.

Given a dict of keys, update the current config from them, validate
it, and return a new config with the updated values
"""
# dct = self.to_dict(omit_none=False)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this probably should be cleaned up


self_merged = cls._merge_dicts(dct, data)
dct.update(self_merged)

adapter_merged = adapter_config_cls._merge_dicts(dct, data)
dct.update(adapter_merged)

# any remaining fields must be "clobber"
dct.update(data)

return dct

@classmethod
def _merge_dicts(cls, src: Dict[str, Any], data: Dict[str, Any]) -> Dict[str, Any]:
"""Mutate input to return merge results.
Expand Down Expand Up @@ -150,30 +172,6 @@ def _merge_dicts(cls, src: Dict[str, Any], data: Dict[str, Any]) -> Dict[str, An
)
return result

def update_from(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there don't appear to be explicit usages of update_from in dbt-adapters: https://github.com/search?q=repo%3Adbt-labs%2Fdbt-adapters%20update_from&type=code

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The signature change here looks like it will have a very minor impact (just to dbt-core). We could release a dbt-common major version bump for this change since it is technically breaking, but that might be overkill given its low usage. At least a minor bump makes sense to me though.

self: T, data: Dict[str, Any], config_cls: Type[BaseConfig], validate: bool = True
) -> T:
"""Update and validate config given a dict.

Given a dict of keys, update the current config from them, validate
it, and return a new config with the updated values
"""
dct = self.to_dict(omit_none=False)

self_merged = self._merge_dicts(dct, data)
dct.update(self_merged)

adapter_merged = config_cls._merge_dicts(dct, data)
dct.update(adapter_merged)

# any remaining fields must be "clobber"
dct.update(data)

# any validation failures must have come from the update
if validate:
self.validate(dct)
return self.from_dict(dct)

def finalize_and_validate(self: T) -> T:
dct = self.to_dict(omit_none=False)
self.validate(dct)
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ dependencies = [
"isodate>=0.6,<0.7",
"jsonschema>=4.0,<5.0",
"Jinja2>=3.1.3,<4",
"mashumaro[msgpack]>=3.9,<4.0",
"mashumaro[msgpack]>=3.15,<4.0",
"pathspec>=0.9,<0.13",
"protobuf>=5.0,<6.0",
"python-dateutil>=2.0,<3.0",
Expand Down
Loading