MyPy support in OSS#990
Conversation
|
👋 @haritamar |
| model_specific_channel_config = model_meta_data.get(CHANNEL_KEY) | ||
|
|
||
| model_specific_channel_config = alert.model_meta.get(CHANNEL_KEY) | ||
| if model_specific_channel_config: |
There was a problem hiding this comment.
A bit unsure if I'm missing anything here, but it does look like model_meta is always parsed in __init__ and is already a dict at this point
The only place I had to fix is the mock data in the tests
…rn precommit workflow to py 3.8
haritamar
force-pushed
the
ele-1267-add-mypy-to-the-oss
branch
from
ba76ae4 to
7c2b588
Compare
| quiet: bool = False, | ||
| **kwargs, | ||
| ) -> list: | ||
| if self.profiles_dir is None: |
There was a problem hiding this comment.
Doesn't it use dbt's default in this case?
I think this is supported.
There was a problem hiding this comment.
I recall this was broken in dbt 1.5
The profiles dir is parsed externally, so the config is already assumed to contain it.
I already changed back then the __init__ function to make profiles_dir mandatory (only for the slim runner), but couldn't convince mypy here that it can't be None so added this check
| sample_data = test_result_db_row.sample_data if not disable_samples else None | ||
| if test_result_db_row.test_type == "dbt_test": | ||
| if sample_data is not None and not isinstance(sample_data, list): | ||
| # Sanity check, shouldn't happen |
There was a problem hiding this comment.
Don't really like this.
If it shouldn't happen there shouldn't be a check for it.
I think it means we're doing something wrong.
There was a problem hiding this comment.
mypy has no way of knowing that specifically for dbt_test it must be a list, and for dimension anomalies it must be a dict.
So this is a way to narrow down the type
Do you think cast is better in such cases?
There was a problem hiding this comment.
Changing to cast
2 participants
No description provided.