Loosen type on replaceable_relation and renameable_relation and provide guidance in docstrings #8647
Conversation
…de guidance in docstrings
|
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. |
…de guidance in docstrings
…de guidance in docstrings
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #8647 +/- ##
=======================================
Coverage 86.56% 86.56%
=======================================
Files 175 175
Lines 25595 25596 +1
=======================================
+ Hits 22156 22157 +1
Misses 3439 3439
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
core/dbt/adapters/base/relation.py
Outdated
|
|
||
| @property | ||
| def can_be_replaced(self) -> bool: | ||
| return self.type in self.replaceable_relations | ||
| return self.type in self.replaceable_relations if self.type else False |
There was a problem hiding this comment.
not sure we need this, if self.type is None it should evaluate to False anyways
There was a problem hiding this comment.
I was appeasing the mypy overlord. However, I no longer need it since I backed off of Iterable to Union[Tuple, FrozenSet]. I did that because a general Iterable is not serializable (for mashumaro).
…de guidance in docstrings
mikealfare
deleted the
feature/materialized-views/relation-registration
branch
Problem
The typing on
renameable_relationsandreplaceable_relationsis overly prescriptive. The only requirement is that the relations in this class variable are contained in an object that defines__contains__and that said object is immutable (frozen dataclass).TupleandFrozenSetare both valid examples, and an adapter maintainer may prefer one or the other, or some third option.Solution
Update the type to
Iterable[str]and make updates in the methods to account forNone.Checklist