Feat(sqlmesh_dbt): Select based on dbt name, not sqlmesh name#5420
Feat(sqlmesh_dbt): Select based on dbt name, not sqlmesh name#5420
Conversation
78f5530 to
9003aaa
Compare
sqlmesh/core/context.py
Outdated
| load: bool = True, | ||
| users: t.Optional[t.List[User]] = None, | ||
| config_loader_kwargs: t.Optional[t.Dict[str, t.Any]] = None, | ||
| dbt_mode: bool = False, |
There was a problem hiding this comment.
I called this generically dbt_mode instead of something more specific to selectors incase we discovered other behaviour that we wanted to attach to it
There was a problem hiding this comment.
I wonder if we need the extra flag instead of using the project_type, for when self._project_type == c.DBT
There was a problem hiding this comment.
actually never mind I realise this is for when someone runs sqlmesh_dbt rather than a dbt project with the native sqlmesh commands so this is indeed needed
There was a problem hiding this comment.
I don't like this flag at all. Can we instead have a base Selector class and 2 implementations and then we can configure the implementation used when creating a Context instance.
There was a problem hiding this comment.
Sure, i've refactored Selector to be an abstract base class and then implemented it as NativeSelector and DbtSelector
0007661 to
e551d67
Compare
9003aaa to
f5ce33c
Compare
| fqn | ||
| for fqn, model in all_models.items() | ||
| if fnmatch.fnmatchcase(model.name, node.this) | ||
| if fnmatch.fnmatchcase(self._model_name(model), node.this) |
There was a problem hiding this comment.
I see the wildcard matching happens outside _pattern_to_model_fqns which seems straightforward and should work, but could you add a test for this as well to ensure nothing breaks in the future if someone modifies this? unless if I missed it and there is a wildcard test for dbt model names already
There was a problem hiding this comment.
Well spotted, I added some tests to exercise this codepath based on the dbt docs.
Note that the patterns currently need a * in them to trigger this branch (even though ?, [ and ] could be used as well) so this will need more work in future if we find people using these in the wild
themisvaltinos
left a comment
There was a problem hiding this comment.
lrgtm! I was wondering of ways for the _pattern_to_model_fqns logic to be reduced to a regex, but it will look complicated and I do like your approach a lot better, it is more readable and makes it clear what is happening
f5ce33c to
5e43166
Compare
5e43166 to
1e6cf07
Compare
|
Just noticed it implemented #3583 which was quite painful! Thanks 🙌 |
This builds on the dbt node info exposed in #5412 to enable its usage in the selector engine.
Previously, to use
sqlmesh_dbt --select, you had to use the sqlmesh-native model names.This PR:
dbt_modeflag to theContextto pass to theSelectorenginesqlmesh_dbtCLI when constructing theContextThe selectors still resolve to SQLMesh native fqn's so everything continues to match up downstream, they just match based on dbt fqn instead of sqlmesh model name